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

Pad a vector's items #238

Open
axelkar opened this issue Nov 15, 2023 · 5 comments · May be fixed by #250
Open

Pad a vector's items #238

axelkar opened this issue Nov 15, 2023 · 5 comments · May be fixed by #250
Labels
enhancement New feature or request

Comments

@axelkar
Copy link

axelkar commented Nov 15, 2023

#[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
Copy link
Author

axelkar commented Nov 15, 2023

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

@csnover csnover added the enhancement New feature or request label Nov 26, 2023
@csnover
Copy link
Collaborator

csnover commented Nov 26, 2023

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
Copy link

czaloj commented Mar 3, 2024

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
Copy link

czaloj commented Mar 3, 2024

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
Copy link

czaloj commented Mar 6, 2024

PTAL #250 @csnover @jam1garner

@csnover csnover linked a pull request Nov 27, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants