qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 04/12] hbitmap: Fix merge when b is empty, and r


From: John Snow
Subject: Re: [Qemu-devel] [PATCH 04/12] hbitmap: Fix merge when b is empty, and result is not an alias of a
Date: Thu, 20 Jun 2019 12:13:45 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0


On 6/20/19 11:39 AM, Max Reitz wrote:
> On 20.06.19 03:03, John Snow wrote:
>> Nobody calls the function like this currently, but we neither prohibit
>> or cope with this behavior. I decided to make the function cope with it.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>>  util/hbitmap.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>> index 7905212a8b..45d1725daf 100644
>> --- a/util/hbitmap.c
>> +++ b/util/hbitmap.c
>> @@ -781,8 +781,9 @@ bool hbitmap_can_merge(const HBitmap *a, const HBitmap 
>> *b)
>>  }
>>  
>>  /**
>> - * Given HBitmaps A and B, let A := A (BITOR) B.
>> - * Bitmap B will not be modified.
>> + * Given HBitmaps A and B, let R := A (BITOR) B.
>> + * Bitmaps A and B will not be modified,
>> + *     except when bitmap R is an alias of A or B.
>>   *
>>   * @return true if the merge was successful,
>>   *         false if it was not attempted.
>> @@ -797,7 +798,9 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, 
>> HBitmap *result)
>>      }
>>      assert(hbitmap_can_merge(b, result));
>>  
>> -    if (hbitmap_count(b) == 0) {
>> +    if ((!hbitmap_count(a) && result == b) ||
>> +        (!hbitmap_count(b) && result == a) ||
>> +        (!hbitmap_count(a) && !hbitmap_count(b))) {
>>          return true;
>>      }
> 
> The rest of this function completely overwrites the @result bitmap.
> Therefor, @result does not need to be cleared when calling this function.
> 
> Therfore, hbitmap_merge(hbitmap_alloc(), hbitmap_alloc(), output) should
> actually clear @output, I think.
> 
> Max
> 

Ah, wellp, you're right. That'd be the second problem with this function.

It used to be strictly A = A | B, but we changed it -- very incompletely
-- to R = A | B; which explains these two bugs.

Thanks.



reply via email to

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