Re: auto merging extents

[Pádraig Brady]

On 09/03/11 18:23, Jim Meyering wrote:
> Pádraig Brady wrote:
>> -  i--;
>> -  if (scan->ext_info[i].ext_flags & FIEMAP_EXTENT_LAST)
>> +  scan->ei_count = si;
>> +
>> +  si--;
>> +  if (scan->ext_info[si].ext_flags & FIEMAP_EXTENT_LAST)
>>      {
>>        scan->hit_final_extent = true;
>>        return true;
>>      }
>>
>> +  i--;
>>    scan->scan_start = fm_extents[i].fe_logical + fm_extents[i].fe_length;
> 
> The above is all fine, but unless you know that scan->ei_count
> is always positive, seeing "i" and "si" used as indices right
> after being decremented may make you think there's a risk
> of accessing some_buffer[-1].
> 
> What do you think about adding an assertion, either on scan->ei_count
> before the loop, or on i and/or si after it?

Yes, it's not immediately obvious that i and si >= 1,
but it's guaranteed by the early return when fiemap->fm_mapped_extents==0.
Because of that return, an assert is overkill I think.
Actually I think we can simplify further by just
using a pointer to the last extent_info entry we're updating.

I also noticed that we shouldn't care about
the FIEMAP_EXTENT_LAST flag when merging.

Both changes are in the latest version attached.

cheers,
Pádraig.

cp-merge-extents.diff
Description: Text Data