This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 7/11] Add BFIN_MAX_REGISTER_SIZE


> 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.


>>   enum register_status status;
>> 
>>   if (regnum != BFIN_CC_REGNUM)
>> @@ -710,7 +710,7 @@ static void
>> bfin_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
>> 			    int regnum, const gdb_byte *buffer)
>> {
>> -  gdb_byte *buf = (gdb_byte *) alloca (MAX_REGISTER_SIZE);
>> +  gdb_byte *buf = (gdb_byte *) alloca (BFIN_MAX_REGISTER_SIZE);
> 
> -- 
> Yao (齐尧)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]