Re: [PATCH 1/6] : Commonise various target-descriptions.c functions

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

> On 19 Jan 2018, at 23:05, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> On Tue, Jan 16, 2018 at 9:50 AM, Alan Hayward <Alan.Hayward@arm.com> wrote:
>> This patch simply moves functionality from target-descriptions.c
>> to the common files arch/tdesc.c and arch/tdesc.h.
>> No functionality is changed.
>> This will allow usage by gdbserver.
>> The "#ifndef GDBSERVER" around the functions in arch/tdesc.h will be removed
>> in the next patch.
>> 
> 
> It is not right to move everything to arch/ and remove unused bits.  GDB now
> is able to visit different tdesc_element, you want to do the same in
> GDBserver, and share the code in arch/.  However, tdesc stuff in GDB and
> GDBserver are quite different, so the 1st step, IMO, is to make GDBserver
> more similar to GDB.  You can change each tdesc-related struct in each
> patch, and refactor GDBserver bits.  Then, only move *needed* structures
> to arch/, and move one structure in one patch each, if possible.
> 

Did you want me to change the ordering of the patches in the series or did you
want me to reduce the amount of code that gets moved into arch/ ?

When I wrote the whole patch series, I only moved across the functions I needed
from gdb to arch/. It turns out that I needed to move a lot of functions. Without
complicating the code, I think I need everything I moved into arch/

After I had finished writing my code, I looked at the ways of splitting it up into a
series. The simplest method was to put all the moving of functions into the first
patch. I agree that this makes the first patch a little odd to review. My reasoning
was to keep changes to the moved code separate from the moving.

I can look at making a new version of 2/6 the first patch, and then follow with
smaller versions of this patch? The final change across the whole series would
be the same. 


Alan.