jam1garner / binrw

A Rust crate for helping parse and rebuild binary data using ✨macro magic✨.
https://binrw.rs
MIT License
585 stars 35 forks source link

Pad a vector's items #238

Open axelkar opened 10 months ago

axelkar commented 10 months ago
#[derive(BinRead)]
#[br(little)]
// Is 7+ bytes but needs to get padded to `arbitrary`
pub struct Bar {
  f1: u8,
  f2: u16,
  len: u32,
  #[br(count = len)]
  v: Vec<u8>
}
#[derive(BinRead)]
pub struct Foo {
  arbitrary: u8,
  len: u8,
  #[br(pad_size_to = arbitrary, count = len)]
  pub list: Vec<Bar>
}

// passing [
//   12, 2,
//   1, 2, 0, // f1, f2
//   1, 0, 0, 0, // len
//   42, // list
//   0, 0, 0, 0, // padding
//   4, 5, 0, // f1, f2
//   2, 0, 0, 0, // len
//   41, 43 // list
//   0, 0, 0 // padding
// ]
// should return Foo { arbitrary: 8, len: 2, list: vec![Bar {f1: 1, f2: 2, len: 1, list: vec![42]}] }

This doesn't currently work

axelkar commented 10 months ago

As an alternative a directive could be added to pad a whole struct, which in my example is Bar.

csnover commented 10 months ago

Hi, sorry for the delay. pad_size_to (or any field directive, really) only applies to a field itself, not to items within a collection. I can see how pad_size_to on a struct might be useful. In the interim, you should be able to pass arbitrary as an argument and then pad the last field, something like this (untested) code:

#[derive(BinRead)]
#[br(import(arbitrary: u8), little)]
// Is 7+ bytes but needs to get padded to `arbitrary`
pub struct Bar {
  f1: u8,
  f2: u16,
  len: u32,
  #[br(count = len, pad_after = u32::from(arbitrary) - len - 7)]
  v: Vec<u8>
}
#[derive(BinRead)]
pub struct Foo {
  arbitrary: u8,
  len: u8,
  #[br(args { count: len.into(), inner: (arbitrary,) })]
  pub list: Vec<Bar>
}
czaloj commented 7 months ago

I've been using this functionality recently too and I don't believe that it's a viable solution to expect developers to write reliable code by manually computing sizes of fields. My current workaround so far has been to create a padded wrapper:

#[derive(BinRead, Debug)]
struct TestList {
    n: u8,
    s: u8,
    #[br(args { count: n as usize, inner: binrw::args!{ padding: s } })]
    elements: Vec<Padded<Element>>,
}

#[derive(BinRead, Debug)]
#[br(little, import {
    padding: u8,
})]
struct Padded<T>
where
    T: BinRead<Args<'static> = ()> + std::fmt::Debug + Sized + Default,
{
    #[br(pad_size_to = padding)]
    inner: T,
}

and then add a Deref on top of that. Obviously this is not feature complete with correctly passing args through to inner members.

czaloj commented 7 months ago

I spent a couple minutes making a draft of changes, hopefully someone with more insight can polish it up? https://github.com/jam1garner/binrw/compare/master...czaloj:struct_pad_size?expand=1

czaloj commented 7 months ago

PTAL https://github.com/jam1garner/binrw/pull/250 @csnover @jam1garner