sharksforarms / deku

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

read_with_predicate: stop when empty #380

Closed vhdirk closed 11 months ago

vhdirk commented 11 months ago

When setting a predicate, I have a situation where the bitvec is empty, yet it still attempts to start reading the next element. It seems not too weird to assume that we should stop reading when the last element completed successfully and there's nothing more to process left.

sharksforarms commented 11 months ago

Thanks for the PR! I think having this pass instead of error may be confusing as we expect the predicate to finish as true.

In your binary format, is there a size or another field that indicates there is data to be read into your Vec? Normally there is something like this to prevent reading arbitrary data.

Here's an example using cond

use deku::prelude::*;
use std::convert::TryFrom;

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct {
    size: u8,
    #[deku(cond = "*size > 0", until = "|v: &u8| *v == 0xDD")]
    data: Vec<u8>,
}

fn main() {
    env_logger::init();
    {
        let test_data: Vec<u8> = [0x00].to_vec();
        let ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
        assert_eq!(
            TestStruct {
                size: 0x00,
                data: vec![]
            },
            ret_read
        );
    }
    {
        let test_data: Vec<u8> = [0x03, 0xCC, 0xAA, 0xBB].to_vec();
        let ret_read = TestStruct::try_from(test_data.as_ref()).unwrap();
        assert_eq!(
            TestStruct {
                size: 0x03,
                data: vec![0xCC, 0xAA, 0xBB]
            },
            ret_read
        );
    }
}
vhdirk commented 11 months ago

You're quite right. The protocol I'm implementing does have a size field, so I've refactored my code to reflect that. However, the size field is at the beginning of the frame, whereas the array I'm reading is somewhere further in the message.

Quite rudimentary, it looks like this:

| frame_size | control | [T; frame_size - sizeof(control)]  |

with the added caveat that the control field itself is also not a fixed size, its size depends on some internal flags. Do you have an idea from the top of your head how I could tackle this (elegantly) ?

PS: I'm using count and cond extensively here: https://github.com/vhdirk/dash7-rs

sharksforarms commented 11 months ago

You're quite right. The protocol I'm implementing does have a size field, so I've refactored my code to reflect that. However, the size field is at the beginning of the frame, whereas the array I'm reading is somewhere further in the message.

Quite rudimentary, it looks like this:

| frame_size | control | [T; frame_size - sizeof(control)]  |

with the added caveat that the control field itself is also not a fixed size, its size depends on some internal flags. Do you have an idea from the top of your head how I could tackle this (elegantly) ?

PS: I'm using count and cond extensively here: https://github.com/vhdirk/dash7-rs

Is this (somewhat) representative of your format?

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestControl {
    size: u8,
}

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct {
    size: u8,
    control: TestControl,
    #[deku(cond = "(*size - control.size) > 0", until = "|v: &u8| *v == 0xDD")]
    data: Vec<u8>,
}
vhdirk commented 11 months ago

No, it's somewhat more involved than that:

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestControl {
   #[deku(bits="1", pad_bits_after="7")]
   flag: bool;

    #[deku(cond = "*flag")]
    value: Option<u16>,
}

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct {
    size: u8,
    control: TestControl,
    #[deku(cond = "(*size - control.calculated_packed_size() ) > 0", until = "|v: &u8| *v == 0xDD")]
    data: Vec<u8>,
}

In this simple example, I could check TestControl::flag again in the cond attribute in TestStruct, but in the protocol at hand there would simple be too many variables to properly do this. I reckon I'd a placeholder along the lines of deku::bytes_read that would propagate the number of bytes read up until that point...

sharksforarms commented 11 months ago

No, it's somewhat more involved than that:

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestControl {
   #[deku(bits="1", pad_bits_after="7")]
   flag: bool;

    #[deku(cond = "*flag")]
    value: Option<u16>,
}

#[derive(PartialEq, Debug, DekuRead, DekuWrite)]
struct TestStruct {
    size: u8,
    control: TestControl,
    #[deku(cond = "(*size - control.calculated_packed_size() ) > 0", until = "|v: &u8| *v == 0xDD")]
    data: Vec<u8>,
}

In this simple example, I could check TestControl::flag again in the cond attribute in TestStruct, but in the protocol at hand there would simple be too many variables to properly do this. I reckon I'd a placeholder along the lines of deku::bytes_read that would propagate the number of bytes read up until that point...

There is deku::byte_offset which is what you describe, would that help your use case?

https://docs.rs/deku/latest/deku/#internal-variables-and-previously-read-fields

vhdirk commented 11 months ago

Oh, not sure how I missed that. byte_offset is perfect! thanks!