Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (2/4)

[Pedro Alves <palves@redhat.com>]

On 05/26/2017 04:30 PM, Alan Hayward wrote:
> 
>> On 26 May 2017, at 11:26, Pedro Alves <palves@redhat.com> wrote:
> 
> 
>> Another issue with the test is that by relying on
>> extract_(un)signed_integer, the test can't notice if the
>> zero / sign extension in copy_integer_bytes filled the right
>> number of bytes:
>>
>>> +  do_cint_test (0xffffffffffffbeef, 2, 0xbeef, 3);
>>
>> I.e., above, the last test _must_ not write more than
>> two bytes to the destination, so those leading
>> 0xFFs before "beef" are artificial.
>>
>> Passing ULONGEST as destination value avoids that problem, making
>> you write that last test as:
>>
>>   do_cint_test (0x000000000000beef, 2, 0xbeef, 3);
>>
> 
> To make that work, I also had to make extract use the unsigned
> version. So now all the stores and extracts are working on unsigned data.

Makes sense.
> All the above fixed up.
> After doing this, I noticed it was easier to combine the two test functions
> into one.

Agreed.

> 
> 
> Tested on a --enable-targets=all build using make check with board files
> unix and native-gdbserver.
> I do not have a MIPS machine to test on.

I think the unit tests now give us sufficient assurance.

> Ok to commit?

Yes, with nit below addressed.

> +static void
> +copy_integer_to_size_test ()
> +{
> +  /* Destination is bigger than the source, which has the signed bit unset.  */
> +  do_cint_test (0x12345678, 0x12345678, 8, 0x12345678, 4);
> +  do_cint_test (0x345678, 0x345678, 8, 0x12345678, 3);
> +  do_cint_test (0x5678, 0x5678, 2, 0x12345678, 3);

The third test does not agree with the comment - destination is
smaller than source.

> +
> +  /* Destination is bigger than the source, which has the signed bit set.  */
> +  do_cint_test (0xdeadbeef, 0xffffffffdeadbeef, 8, 0xdeadbeef, 4);
> +  do_cint_test (0xadbeef, 0xffffffffffadbeef, 8, 0xdeadbeef, 3);
> +  do_cint_test (0xbeef, 0xbeef, 2, 0xdeadbeef, 3);

Ditto.

Re-sort to a separate block, and/or update comments, and this
is good to go.

Thanks,
Pedro Alves