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

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On April 3, 2024 2:18 pm, Christian Ebner wrote:
> 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.

but.. payload_consumed only ever goes up? if the offset then jumps back
to a position before the payload_consumed counter, this will underflow
(to_skip is unsigned)?

>>> +                        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

ah, yeah, that makes sense.. I think the only way to avoid it is to
inline skip_payload here (it's the only call site anyway).