Skip to content
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

Validate EbmlId on creation/Don't set the size on creation #177

Closed
robUx4 opened this issue Dec 20, 2023 · 3 comments
Closed

Validate EbmlId on creation/Don't set the size on creation #177

robUx4 opened this issue Dec 20, 2023 · 3 comments
Labels
api-break breaks the API (e.g. programs using it will have to adjust their source code)

Comments

@robUx4
Copy link
Contributor

robUx4 commented Dec 20, 2023

The EbmlId class is very loose. It's possible to create an EbmlId with a bogus size and use it in a class without anyone noticing. We just assume the values are correct in the spec/custom code.

There are easy tools like gcc/clang clz (https://en.cppreference.com/w/cpp/numeric/countl_zero in C++20) and even in MSVC (https://stackoverflow.com/questions/355967/how-to-use-msvc-intrinsics-to-get-the-equivalent-of-this-gcc-code) that can quickly find the proper size. We should not have to set the size manually at all.

The constructors only allows 4 byte values so we already have constraints on the max ID size. We just need to generate the length automatically. EbmlId should be only created once per class. After that they should be used as const references. So the cost on creation shouldn't be too much.

The spectool generates the ID size based on the size of the id attribute string.

@robUx4 robUx4 added the api-break breaks the API (e.g. programs using it will have to adjust their source code) label Dec 20, 2023
@robUx4
Copy link
Contributor Author

robUx4 commented Dec 20, 2023

Maybe a IsValid() method would be enough and we can assert in the constructor that it is indeed valid. That would be enough to check all used IDs are valid in debug builds.

@mbunkus
Copy link
Contributor

mbunkus commented Dec 20, 2023

Checking if an ID is valid would indeed be a great function to have. Would need some kind of top-level EbmlContext to check against, I guess; with Matroska that would be the one from KaxSegment.

@robUx4
Copy link
Contributor Author

robUx4 commented Feb 26, 2024

Done in #230

@robUx4 robUx4 closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break breaks the API (e.g. programs using it will have to adjust their source code)
Projects
None yet
Development

No branches or pull requests

2 participants