Re: [PATCH 7/11] Add BFIN_MAX_REGISTER_SIZE

[Alan Hayward <Alan.Hayward@arm.com>]

> On 5 Apr 2017, at 14:50, Andreas Schwab <schwab@suse.de> wrote:
> 
> On Apr 05 2017, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
>>> On 5 Apr 2017, at 11:28, Yao Qi <qiyaoltc@gmail.com> wrote:
>>> 
>>> Alan Hayward <Alan.Hayward@arm.com> writes:
>>> 
>>>> diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
>>>> index 3df1ba387a323dc6827b1189432f8877d1833184..9b45633cab15b8e0adb0d51a2fa650dc2bc6339b 100644
>>>> --- a/gdb/bfin-tdep.c
>>>> +++ b/gdb/bfin-tdep.c
>>>> @@ -689,7 +689,7 @@ static enum register_status
>>>> bfin_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
>>>> 			   int regnum, gdb_byte *buffer)
>>>> {
>>>> -  gdb_byte *buf = (gdb_byte *) alloca (MAX_REGISTER_SIZE);
>>>> +  gdb_byte *buf = (gdb_byte *) alloca (BFIN_MAX_REGISTER_SIZE);
>>> 
>>> Why don't you do "gdb_byte buf[4];"?  It is only for CC register which
>>> is 32-bit.
>>> 
>> 
>> Is it not clearer code to add and use a macro rather than a magic number ?
>> 
>> It’s also not obvious anywhere that the astat register is 32bits. I had to go
>> digging inside regformats/reg-bfin.dat before I found it out.
>> 
>> Given that BFIN_MAX_REGISTER_SIZE is also 4, it compiles to the same size anyway.
> 
> Since BFIN_MAX_REGISTER_SIZE is a constant you don't need alloca either
> way.
> 

Sorry, yes, I meant to say:

Why not use:
gdb_byte buf[BFIN_MAX_REGISTER_SIZE];

Rather than
gdb_byte buf[4];

Given that a macro is clearer than a magic number.

Alan.