Skip to content

der: add indefinite length #1820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dishmaker
Copy link
Contributor

We can use u32::MAX as indefinite variant

@tarcieri
Copy link
Member

tarcieri commented May 6, 2025

I've been trying to work on indefinite lengths but in a way that avoids the need to have dummy lengths or an IndefiniteLength type

@dishmaker
Copy link
Contributor Author

I tried a trick with NonZero<u32> but really what we need is NonMax<u32>

@dishmaker
Copy link
Contributor Author

Because

/// This enum is 4 bytes
enum LengthInner {
    Definite(NonZero<u32>),
    Indefinite,
}

@tarcieri
Copy link
Member

tarcieri commented May 6, 2025

@dishmaker that enum looks a lot like IndefiniteLength (which I just removed in #1807) though we still need to support definite lengths of zero

@dishmaker
Copy link
Contributor Author

dishmaker commented May 6, 2025

Yeah, so we need NonMax<u32> and change Length to a 4-byte enum (to allow compiler optimizations)

https://docs.rs/nonmax/latest/nonmax/struct.NonMaxU32.html

@tarcieri
Copy link
Member

tarcieri commented May 6, 2025

@dishmaker I definitely don't want to pull in an extra dependency for that. der is otherwise dependency-free.

The newtype can enforce its own maximum like it did before #1726

@dishmaker dishmaker force-pushed the dishmaker/der_indefinite_length branch from 0b79ef6 to 47a1cd9 Compare May 12, 2025 17:12
@dishmaker dishmaker changed the title demo: der: add indefinite length der: add indefinite length May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants