Re: [RFA 2/6] gmp-utils: Convert the read/write methods to using gdb::array_view

Simon Marchi
2020-11-17T14:39:35Z

On 2020-11-16 6:05 p.m., Pedro Alves wrote:
> On 11/16/20 12:52 AM, Simon Marchi wrote:
>> On 2020-11-15 3:49 a.m., Joel Brobecker wrote:
>>> This commit changes the interfaces of some of the methods declared
>>> in gmp-utils to take a gdb::array_view of gdb_byte instead of a
>>> (gdb_byte *, size) couple.
>>>
>>> This makes these methods' API probably more C++-idiomatic.
>>> With the way things are structured, this change introduces a minor
>>> extra complication at the point of call of these methods, since
>>> the data available there is not in the form of an array_view,
>>> and thus the array_view needs to be constructed on the spot.
>>
>> I'd suggest using gdb::make_array_view (ptr, len) instead of the
>> array_view constructor. This way, you don't need to specify the
>> template type, it's automatically deduced, so it's a bit less verbose.
>> Otherwise, LGTM.
>
> Note gdb::make_array_view is only necessary if the type of len
> is not size_t. If it is size_t, then the shorter "{ptr, len}" works.
>
> As an illustration:
>
> --- i/gdb/unittests/gmp-utils-selftests.c
> +++ w/gdb/unittests/gmp-utils-selftests.c
> @@ -95,7 +95,7 @@ gdb_mpz_as_integer ()
>
> template<typename T>
> void
> -store_and_read_back (T val, int buf_len, enum bfd_endian byte_order,
> +store_and_read_back (T val, size_t buf_len, enum bfd_endian byte_order,
> gdb_mpz &expected, gdb_mpz &actual)
> {
> gdb_byte *buf;
> @@ -109,8 +109,7 @@ store_and_read_back (T val, int buf_len, enum bfd_endian byte_order,
> mpz_set (actual.val, expected.val);
> mpz_sub_ui (actual.val, actual.val, 500);
>
> - actual.read (gdb::array_view<const gdb_byte> (buf, buf_len),
> - byte_order, !std::is_signed<T>::value);
> + actual.read ({buf, buf_len}, byte_order, !std::is_signed<T>::value);
> }
>

Well, +1 for using that.

Simon