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

Missing magic changed to be a unique error vs a parsing error #440

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Nub
Copy link

@Nub Nub commented May 19, 2024

This change is needed so parsers that work on stream data can know wether to advance into the buffer to find a valid message without having to manually search for magic bytes before feeding the DekuRead object.

…arsers for framing data from streams is easier
Copy link
Collaborator

@wcampbell0x2a wcampbell0x2a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Check the CI, there are some other errors.

src/error.rs Outdated Show resolved Hide resolved
deku-derive/src/macros/deku_read.rs Outdated Show resolved Hide resolved
@Nub
Copy link
Author

Nub commented May 19, 2024

Thanks! Check the CI, there are some other errors.

Will do! Shoulda marked this as a draft while I was working on that, apologies.

@Nub
Copy link
Author

Nub commented May 19, 2024

What version of the toolchain is being used, compiler outputs being used for tests without pinning a version is what's causing some of these CI failures. I had been on a nightly 1.80 which apparently changes the output of the compiler, fun fun!

@wcampbell0x2a
Copy link
Collaborator

What version of the toolchain is being used, compiler outputs being used for tests without pinning a version is what's causing some of these CI failures. I had been on a nightly 1.80 which apparently changes the output of the compiler, fun fun!

Yah I usually use nightly and then use cargo +stable test before commits. The toolchain used in CI is the stable one.

/// Deku errors
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub enum DekuError {
/// Parsing error when reading
Incomplete(NeedSize),
/// Failed to find magic
Framing(NeedMagic),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would prefer the name relate to magic, unless we plan on expanding this to be an enum with different variants

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.

None yet

3 participants