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

[Christian Ebner <c.ebner@proxmox.com>]

On 4/3/24 12:38, Fabian Grünbichler wrote:
>> +
>> +                if let Some(payload_input) = self.payload_input.as_mut() {
> 
> this condition (cted below)
> 
>> +                    if seq_read_position(payload_input)
>> +                        .await
>> +                        .transpose()?
>> +                        .is_none()
>> +                    {
>> +                        // Skip payload padding for injected chunks in sequential decoder
>> +                        let to_skip = payload_ref.offset - self.payload_consumed;
> 
> should we add a check here for the invariant that offsets should only
> ever be increasing? (and avoid an underflow for corrupt/invalid archives
> ;))

This is called by both, seq and random access decoder instances, so that 
will not be possible I guess.

> 
>> +                        self.skip_payload(to_skip).await?;
>> +                    }
>> +                }
>> +
>> +                if let Some(payload_input) = self.payload_input.as_mut() {
> 
> and this condition here are the same?

While this seems just duplicate, it makes the borrow checker happy as 
otherwise it complains that the &mut self borrow of the skip_payload 
call and the following seq_read_entry call taking the payload_input are 
in conflict.
I am happy for any hint on how to make the borrow checker happy without 
having to perform the if check two time
> 
>> +                    let header: u64 = seq_read_entry(payload_input).await?;
> 
> why not read a Header here?

Yeah, definitely better to read the full header, and then check against 
the htype and content_size(). Will change this for the next version.

> 
> 
> then these could use the size helpers of Header ;)