[PATCH v4 02/10] Make gdbserver reg_defs a vector of objects

[Alan Hayward]

> On 23 Mar 2018, at 15:35, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> 
> On 2018-03-23 10:54, Alan Hayward wrote:
>>> Please keep the curly braces for the for, as shown here:
>>> https://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html#index-multiple-variables-in-a-line
>>> Can you put the moving of this function/making it static/removing the declaration
>>> in its own patch?  It's pre-approved, with that fixed.
>> Ok, I’ve pushed this bit as requested:
> 
> Thanks!
> 
>> New version updated with all of the above.
>> Checked this on X86 with make check on target board gdbserver.
>> (Patch 3/10, and possibly others, will need updating too, but should
>> be obvious).
>> Thanks for the review.
>> Alan.
> 
> I had some problems applying your patch, the tabs were replaced with spaces.  You can either fix the settings of your email client, or you can also use git-send-email when sending individual patch updates, like this:
> 
>  git send-email HEAD^ --subject-prefix="PATCH v4.1 02/10" --to <to> --in-reply-to <message-id>
> 
> where message-id can be found in the headers of the message you want to reply to (header Message-ID).  This allows the message to be correctly threaded.  It means the patch will be in a separate email than your response to the reviewer's comments, but I think that's fine.
> 
> Anyway, this particular one wasn't too difficult to fix up by hand.  It LGTM with one nit fixed:
> 

Thanks for the tip

>> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
>> index
>> 262d03c0785f48e83b784b6177c52e2d253a6067..1f7861fd98dd9085c3fe9d7ee4b8396999060265
>> 100644
>> --- a/gdb/regformats/regdef.h
>> +++ b/gdb/regformats/regdef.h
>> @@ -21,6 +21,18 @@
>> struct reg
>> {
>> +  reg ()
>> +    : name (""),
>> +      offset (0),
>> +      size (0)
>> +  {}
>> +
>> +  reg (const char *_name, int _offset, int _size)
> 
> The offset value is always 0 initially, so you can remove it and initialize it to 0.
> 

Reason I added offset here was that in "Commonise tdesc_reg” this code will get merged into init_target_desc().
At that point I’ll be creating a reg and setting and offset at the same time.
This was just so that I didn’t need to touch regdef.h again.

I can still remove offset from this patch if you want - given that I’m not using it yet?


> I think that this patch can also be pushed on its own, it's a good improvement regardless of the rest of the series.
> 
> Thanks,
> 
> Simon