This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]