On 31.07.19 10:54, Max Reitz wrote:
> On 30.07.19 19:56, Eric Blake wrote:
>> On 7/30/19 12:24 PM, Max Reitz wrote:

[...]

>>> +        if (sn->extra_data_size > sizeof(extra)) {
>>> +            /* Store unknown extra data */
>>> +            size_t unknown_extra_data_size =
>>> +                sn->extra_data_size - sizeof(extra);
>>> +
>>> +            sn->unknown_extra_data = g_malloc(unknown_extra_data_size);
>>> +            ret = bdrv_pread(bs->file, offset, sn->unknown_extra_data,
>>> +                             unknown_extra_data_size);
>>
>> We're doing two separate bdrv_pread()s. Would it be better to do a
>> single bdrv_preadv into a vector composed of &extra and
>> &unknown_extra_data, for less I/O?  (Then again, this micro-optimization
>> is probably in the noise in the long run)
> 
> Interesting idea, we could even add the ID and name string into that
> vector.  But I’m not sure whether it’s really useful.
> 
> (I’ll take a look anyway, because it sounds interesting.)

I did, and it was actually really nice.  I liked it.  (I don’t hink the
performance is important, but it was actually just simpler.)

But then it turned out that it won’t work with patch 8, because after
that we may want to skip parts of the unknown extra data – and skipping
doesn’t work with bdrv_preadv()...

(So all the simplicity is lost, and that’s what I was interested in.)

But it was indeed quite cool. :-/

Max

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to