Re: [PATCH [2/2] Convert the unavailable vector to be bit, not byte, based.

[Andrew Burgess <aburgess@broadcom.com>]

On 05/12/2013 4:18 PM, Pedro Alves wrote:
> On 12/04/2013 03:26 PM, Andrew Burgess wrote:
> 
>>  
>>        /* Compare the _available_ contents.  */
>> -      if (memcmp (val1->contents + offset1,
>> -		  val2->contents + offset2,
>> -		  l1) != 0)
>> +      if (memcmp (val1->contents + (offset1 / TARGET_CHAR_BIT),
>> +		  val2->contents + (offset2 / TARGET_CHAR_BIT),
>> +		  (l1 / TARGET_CHAR_BIT)) != 0)
>>  	return 0;
> 
> As memcmp compares bytes, isn't this potentially comparing bits
> at the beginning and end of the values' buffers, when it
> should not?   That is, it looks like the
> 'offset1 % TARGET_CHAR_BIT != 0' and
> '(offset1 + l1) % TARGET_CHAR_BIT' cases should be considered here?
> 

Thanks for the review.  You're right, this isn't doing the correct thing
here, I should have written something like this:


+      if (memcmp (val1->contents + (offset1 / TARGET_CHAR_BIT),
+		  val2->contents + (offset2 / TARGET_CHAR_BIT),
+		  ((l1 + TARGET_CHAR_BIT - 1)/ TARGET_CHAR_BIT)) != 0)

That is round the start down to the nearest byte boundary, and round the
length up to the nearest byte boundary.

I figured that as the value content buffer is always a round number of
bytes then there will always be memory backing the access, and as the
content buffer is zero initialized comparing the "unavailable" bits will
not cause the compare to fail.

Alternatively, I can update to only access the bits we'd like to compare.

Which approach would would be best?

Thanks for your advice,
Andrew