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

Replace EBML parsing error code (u8) with error enum #101

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

FreezyLemon
Copy link
Contributor

This is a suggestion to make the EBML errors better. Feel free to change things or just let me know what you think.

Changes:

  1. Introduce ebml::EbmlError enum to represent errors instead of u8 error codes.
  2. Change some of the error code values (the values 1 and 104 were used ambiguously)
  3. Make the helper function custom_error return a nom:Err (specifically, the recoverable Error variant) wrapped around the ebml::Error instead of just the ebml::Error. This simplifies the function calls.

I thought about #[repr(u8)] for the new enum, but realistically the enclosing ebml::Error struct will be padded anyways, so it probably doesn't matter if the enum is represented as a u16 or even larger in the future.

I'm not sure about the actual error codes. If the numerical values are supposed to be expressive, it would make sense to think of some kind of numbering scheme for the error codes before merging. Otherwise, the numerical values can (IMO) just be removed.

@lu-zero lu-zero merged commit 99e0290 into rust-av:master Mar 29, 2023
@lu-zero
Copy link
Member

lu-zero commented Mar 29, 2023

Thank you!

@FreezyLemon FreezyLemon deleted the custom-errorkind-type branch March 29, 2023 12:10
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