[pbs-devel] [PATCH v3 pxar 07/58] decoder/accessor: add optional payload input stream

[Christian Ebner]

On 4/3/24 12:38, Fabian Grünbichler wrote:

> style nit: for those fns that take input and payload_input, it might
> make sense to order them next to eachother? IMHO it makes the call sites
> more readable, especially in those cases where it's mostly passed along
> ;) for the constructors, I am a bit torn on which variant is nicer.\

Maybe I can do that as a followup, as this already touches so much stuff 
it feels wrong to also move implementations around?

> 
> nit: would be easier to parse if it were
> 
> let range = ..
> if let Some(ref payload_input) = .. {
> ..
> } else {
> ..
> }
> 
> and it would also mesh better with `open_contents_at_range` above.

Ah yes, will change that!

> 
> I am a bit confused by this here - shouldn't all payloads be encoded via
> refs now if we have a split archive? and vice versa? why the second
> condition? and what if I pass a bogus payload input for an archive that
> doesn't contain any references?

Got me on this one, sorry for confusing you here: this was added with 
the intention to not encode files with zero payload size in the payload 
stream, but rather in the metadata stream for space optimization. 
However this caused issues so was removed again on the proxmox-backup 
side, but I forgot to remove it here as well. Will remove this for the 
next version.

> 
> similar here..
> 
> e.g., something like this:
> 
>              let input = if *payload_ref {
>                  if let Some(payload_input) = self.payload_input.as_mut() {
>                      payload_input
>                  } else {
>                      return None;
>                  }
>              } else {
>                  &mut self.input
>              };
>              Some(Contents::new(input, offset, *size))
> 
> although technically we do have an invariant there that we could check -
> we shouldn't encounter a non-ref payload when we have a payload_input..

yes same as above, this was added for the zero payload files.