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

[Alan Hayward <Alan dot Hayward at arm dot com>]

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.