coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: auto merging extents


From: Pádraig Brady
Subject: Re: auto merging extents
Date: Thu, 10 Mar 2011 10:26:30 +0000
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

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.

Attachment: cp-merge-extents.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]