sharksforarms / deku

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

Support for fallible use cases for mutating and serializing fields #457

Closed maxfierrog closed 4 months ago

maxfierrog commented 4 months ago

I was wondering if there was API support for either of the following use cases, or an equivalent thereof:

#[derive(Debug, Default, PartialEq, DekuWrite, DekuRead)]
#[deku(endian = "big")]
struct Test {
  #[deku(bits = 4)]
  field: u8,
}

fn fallible_case_1() -> anyhow::Result<()> {
  let mut x = Test::default();

  // Generated methods?
  x.set_field(17)?; // Fails
  x.set_field(15)?; // Ok

  Ok(())
}

fn fallible_case_2() -> anyhow::Result<()> {
  let mut x = Test::default();
  x.field = 17;
  x.to_bytes()?; // Does not error, alarmingly
  x.to_bytes_checked()?; // Errors because 17 >= 16; is there something like this?
  Ok(())
}
maxfierrog commented 4 months ago

This is particularly concerning because the overflow checks would depend on the value provided to #[deku(bits = X)], which is not an actual value due to being a macro input. So doing something like...

const FIELD_SIZE: usize = 4;

#[derive(Debug, Default, PartialEq, DekuWrite, DekuRead)]
#[deku(endian = "big")]
struct Test {
  #[deku(bits = FIELD_SIZE)]
  field: u8,
}

fn set_field(struct: &mut Test, value: u8) -> anyhow::Result<()> {
  if value > max_value_for(FIELD_SIZE) {
    Err(...)
  } else {
    input.field = value;
    Ok(())
  }
}

...is of course impossible, as having a const FIELD_SIZE as a SSOT for the bit size of the field does not compile. From my POV this means that having some form of checked field setters or to_bytes method would be up to another Deku derive macro.

wcampbell0x2a commented 4 months ago

I opened https://github.com/sharksforarms/deku/pull/458, which should maybe not address all the issues but is a step in the right direction for removing surprising behavior.