Re: [PATCH 5/8] Add aarch64 psuedo help functions

Alan Hayward
2018-06-04T13:13:14Z

Pushed with minor changes as suggested below.

> On 31 May 2018, at 13:06, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> psuedo -> pseudo in the title.

Ok.

>
> On 2018-05-11 06:52 AM, Alan Hayward wrote:
>> Reduce code copy/paste by adding two helper functions for
>> aarch64_pseudo_read_value and aarch64_pseudo_write
>> The patch does not change any functionality.
>>
>> 2018-05-11 Alan Hayward <alan.hayward@arm.com>
>>
>> * aarch64-tdep.c (aarch64_pseudo_read_value_2): New helper func.
>> (aarch64_pseudo_write_2): Likewise.
>> (aarch64_pseudo_read_value): Use helper.
>> (aarch64_pseudo_write): Likewise.
>> ---
>> gdb/aarch64-tdep.c | 173 +++++++++++++++++------------------------------------
>> 1 file changed, 54 insertions(+), 119 deletions(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 7893579d58..003fefb3c9 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -2247,109 +2247,67 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>> return group == all_reggroup;
>> }
>>
>> +/* Inner version of aarch64_pseudo_read_value. */
>> +
>> +static struct value *
>> +aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
>> + int regsize, struct value *result_value)
>
> It's not a big deal, but in other GDB parts we actually often use the _1 suffix
> for the helper functions.

Changed to aarch64_pseudo_read_value_1 and aarch64_pseudo_write_1.

>
> But otherwise, LGTM.
>
> Simon

>
> On 31 May 2018, at 15:57, Pedro Alves <palves@redhat.com> wrote:
>
> On 05/31/2018 01:06 PM, Simon Marchi wrote:
>
>>> +/* Inner version of aarch64_pseudo_read_value. */
>>> +
>>> +static struct value *
>>> +aarch64_pseudo_read_value_2 (readable_regcache *regcache, int regnum_offset,
>>> + int regsize, struct value *result_value)
>>
>> It's not a big deal, but in other GDB parts we actually often use the _1 suffix
>> for the helper functions.
>>
>
> Also, I'd think "Helper for aarch64_pseudo_read_value." would be clearer,
> because "inner version" sounds a bit odd to me.

Updated as suggested. Also updated write function too.

>
>> But otherwise, LGTM.
>
> To me too.
>
> Thanks,
> Pedro Alves

Thanks!
Alan.