On 06/01/2016 01:47 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> Commit 7f8f9ef1 introduced the ability to store a list of >> integers as a sorted list of ranges, but when merging ranges, >> it leaks one or more ranges. It was also using range_get_last() >> incorrectly within range_compare() (a range is a start/end pair, >> but range_get_last() is for start/len pairs), and will also >> mishandle a range ending in UINT64_MAX (remember, we document >> that no range covers 2**64 bytes, but that ranges that end on >> UINT64_MAX have end < begin). >>
>>
>> - if (!list) {
>> - list = g_list_insert_sorted(list, data, range_compare);
>> - return list;
>> + /* Range lists require no empty ranges */
>> + assert(data->begin < data->end || (data->begin && !data->end));
>
> Consider { begin = 0, end = 0 }.
>
> Since zero @end means 2^64, this encodes the (non-empty) range
> 0..2^64-1.
Or else it means an uninitialized range. My argument is that no range
can contain 2^64 bytes, and therefore the only possible range that would
be that large (0..2^64-1) is unrepresentable, therefore, if end == 0,
begin must be non-zero for the range to be valid as an initialized range.
>
> range.h's comment
>
> * Notes:
> * - ranges must not wrap around 0, but can include the last byte ~0x0LL.
> * - this can not represent a full 0 to ~0x0LL range.
>
> appears to be wrong. The actual limitation is "can't represent ranges
> wrapping around zero, and can't represent the empty range starting at
> zero." Would you like to correct it?
I'm not sure what corrections it needs, though.
>
> I'm afraid range.h is too clever by half.
Unfortunately true.
>
>> +
>> + for (l = list; l && range_compare(l->data, data) < 0; l = l->next) {
>> + /* Skip all list elements strictly less than data */
>> }
>
> Let's put the comment before the loop. It describes the whole loop.
> Also makes the emptiness of the body more obvious.
Sure.
>
> I think I could fix up things on commit (assuming we agree on what needs
> fixing).
>
Adding other authors of range.h for their opinions...
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
