This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 26 Feb 2018 10:45:50 +0400
- Subject: Re: [RFA v2 1/4] Sign-extend non-bit-fields in unpack_bits_as_long
- Authentication-results: sourceware.org; auth=none
- References: <20180222203018.23551-1-tom@tromey.com> <20180222203018.23551-2-tom@tromey.com>
Hi Tom,
On Thu, Feb 22, 2018 at 01:30:15PM -0700, Tom Tromey wrote:
> unpack_bits_as_long is documented as sign-extending its result when
> the type is signed. However, it was only doing sign-extension in the
> case where the field was a bitfield -- that is, not when the "bitsize"
> parameter was 0, indicating the size should be taken from the type.
>
> Also, unpack_bits_as_long was incorrectly computing the shift for
> big-endian architectures for the non-bitfield case.
>
> This patch fixes these bugs in a straightforward way. A new selftest
> is included.
>
> 2018-02-22 Tom Tromey <tom@tromey.com>
>
> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> unittests/unpack-selftests.c.
> * unittests/unpack-selftests.c: New file.
> * value.c (unpack_bits_as_long): Fix bugs in non-bitfield cases.
Looks good to me. Just one thing: I think we need to adjust
unpack_bits_as_long's documentation a bit to match the reality of
how this function is called:
/* Unpack a bitfield of the specified FIELD_TYPE, from the object at
VALADDR, and store the result in *RESULT.
The bitfield starts at BITPOS bits and contains BITSIZE bits.
I checked the callers, and none of them really make a distinction between
bitsize = 0 and bitsize != 0. And since it's fairly straightforward to
handlet this case directly in unpack_bits_as_long, I agree this is best.
Pre-approved with this change.
Thanks for adding a self-test!
--
Joel