This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH [2/2] Convert the unavailable vector to be bit, not byte, based.
- From: Andrew Burgess <aburgess at broadcom dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Mon, 9 Dec 2013 21:04:39 +0000
- Subject: Re: [PATCH [2/2] Convert the unavailable vector to be bit, not byte, based.
- Authentication-results: sourceware.org; auth=none
- References: <529F489F dot 7070805 at broadcom dot com> <529F498F dot 7060909 at broadcom dot com> <52A0A760 dot 8000509 at redhat dot 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