sharksforarms / deku

Declarative binary reading and writing: bit-level, symmetric, serialization/deserialization
Apache License 2.0
1.05k stars 54 forks source link

Passing a struct in ctx creates opaque error #408

Open sempervictus opened 5 months ago

sempervictus commented 5 months ago

Using non-adjacent fields to determine and then read enum types per https://github.com/sharksforarms/deku/blob/master/examples/enums.rs appears to get convoluted when endianness and identifiers are in the context. Specifically:

#[deku_derive(DekuRead, DekuWrite)]
#[derive(Debug, PartialEq)]
#[deku(endian = "endian", ctx = "endian: deku::ctx::Endian")]
pub(super) struct Svc {
    pub(super) hdr: SvcHeader,
    #[deku(ctx = "*hdr")]
    pub(super) svc: SvcBody
}
#[deku_derive(DekuRead, DekuWrite)]
#[derive(Debug, PartialEq)]
#[deku(endian = "endian", ctx = "endian: deku::ctx::Endian")]
pub(super) struct SvcHeader {
    pub(super) svc_type: u16,
    pub(super) pkts: u16,
    #[deku(assert = "*tail == 0")]
    pub(super) tail: u32,
}
#[deku_derive(DekuRead, DekuWrite)]
#[derive(Debug, PartialEq)]
#[deku(endian = "endian", ctx = "endian: deku::ctx::Endian, hdr: SvcHeader", id = "hdr.svc_type")]
pub enum SvcBody {
    #[deku(id = "0x0001")]
    FIRST {
...

doesn't compile due to:

151 | #[deku_derive(DekuRead, DekuWrite)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: message: called `Result::unwrap()` on an `Err` value: Error("expected `,`")

being thrown from the SvcBody enum

sempervictus commented 5 months ago

This is caused, somehow by the act of passing a struct instead of a primitive type in the ctx; passing the svc_type as a u16 resolves the compiler error; but this means having to decompose all structs into members if passing something with complex references.

wcampbell0x2a commented 5 months ago

Not at my computer, but could you try out the master branch? I think this is a duplicate of the fix MR here.

sempervictus commented 5 months ago

@wcampbell0x2a - any chance you could merge #387 to master? Best thing since sliced bread for writing parsers for undocumented protocols where you need that "whatever is left over" set of bytes to still get parsed while you figure out the preceding structures/deps. Currently using it so would be swell to not have to comment-out the WIP stuff to test the stuff i think should work :)

wcampbell0x2a commented 5 months ago

Merged

sempervictus commented 5 months ago

Just about there. Trying to pass the struct by reference results in:

169 |     #[deku(ctx = "hdr")]
    |                  ^^^^^ expected `SvcHeader`, found `&SvcHeader`

and trying to do it by value violates the borrow checker:

169 |     #[deku(ctx = "*hdr")]
    |                  ^^^^^^ move occurs because `*hdr` has type `SvcHeader`, which does not implement the `Copy` trait

Deriving Clone, Copy for the header struct seems to satisfy the checker though i'm not entirely sure what side-effects this has if the passed context is distinct from the actual data in the stream.

wcampbell0x2a commented 5 months ago

@sempervictus Checkout https://github.com/sharksforarms/deku/pull/411 for a fix for this. Sorry I didn't have time today to find an actual solution. I think.

sempervictus commented 5 months ago

@wcampbell0x2a thank you, that PR's a brainfull so to speak... Lots of reading ahead on my end to actually figure out the codegen being done by those macros at some point :smile:. That said, does cloning the pat type impact mutability of the enum's contents? If that's rebased off current master i'll give it a whirl when i go back to that piece of code.

If its not too much trouble, and you happen to know - what does the derivation of cloning/copying do for actual mutability of content given that we're working off an underlying byte stream? Protocol machinations such as proxies need to alter those existing streams in-flight (making the inability to use update on member structs mildly confounding/verbose to write without the member struct just using fields) and coupled with the borrow checker's constraints, the process can get "hacky" at times :grin: