[pbs-devel] [PATCH v4 vma-to-pbs 5/6] Add support for streaming the VMA file via stdin

[Filip Schauer]

On 06/03/2024 16:49, Max Carrara wrote:
> Regarding `F: Fn(u8, u64, Option<Vec<u8>>) -> Result<()>`:
> * Why `Fn` and not `FnOnce`? You call this with a closure later on.

It is called multiple times in the for loop in restore_extent.


>> +            let is_zero = blockinfo.mask == 0;
> I'm usually in favour of assigning the result of conditional checks
> to variables first, but is that really necessary here?
>
>>   
>> -        while file_offset < vma_file_size {
>> -            self.vma_file.seek(SeekFrom::Start(file_offset))?;
>> -            let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?;
>> -            file_offset += size_of::<VmaExtentHeader>() as u64;
>> +            let image_chunk_buffer = if is_zero {
> It's only used here after all, and inlining it wouldn't make the code
> more complex at all.

Not necessary but it improves readability in my opinion.


> Also,
> `Vec<u8>` could probably be `&[u8]`, couldn't it?
No, because then the image_chunk_buffer would not live long enough.
The chunk needs to live past func so it can be stored in the
images_chunks HashMap in vma2pbs.rs. This HashMap can hold multiple
chunks before they are sent off as one big chunk to the PBS.