[pbs-devel] [PATCH v3 pxar 06/58] encoder: move to stack based state tracking

[Christian Ebner]

On 4/3/24 11:54, Fabian Grünbichler wrote:
> 
> should we still have some sort of checks here? e.g., when dropping an
> encoder, how should self.finished and self.state look like? IIUC, then a
> dropped encoder should have an empty state and be finished (i.e.,
> `close()` has been called on it).
> 
> or is this simply not relevant anymore because we only create one and
> then drop it at the end (but should we then have a similar mechanism for
> EncoderState?)

The encoder should now be consumed with the `close` call, which takes 
ownership of the encoder and drops it afterwards, so all the state 
checks should happen there.

Previously, the encoder finish consumed the per-directory level encoder 
object, passing possible errors up to the parent implementation, which 
is not possible now since there is only one encoder instance. I did not 
want to panic here as the checks should be done in the close now, so the 
Drop implementation was removed.

Not sure what to check in a Drop implementation the EncoderState. What 
did you have in mind for that? Note that errors get propagated to the 
parent state in the encoder finish calls now.

>> +    fn output_state(&mut self) -> io::Result<(&mut T, &mut EncoderState)> {
>> +        Ok((
>> +            self.output.as_mut(),
>> +            self.state
>> +                .last_mut()
>> +                .ok_or_else(|| io_format_err!("encoder state stack underflow"))?,
>> +        ))
>> +    }
>> +
> 
> we could have another helper here that also returns the Option<&mut T>
> for payload_output (while not used as often, it might still be a good
> idea for readability):

Okay, yes I can add that.