Re: [PATCH] Share more common target structures between gdb and gdbserver

[Pedro Alves <palves@redhat.com>]

On 07/31/2013 07:43 PM, Luis Machado wrote:
> On 07/30/2013 02:47 PM, Pedro Alves wrote:
>> On 07/16/2013 07:49 PM, Luis Machado wrote:
>>> Hi,
>>>
>>> While doing some research about the remote fork following feature, i
>>> noticed there was some duplication of target data structures for GDB and
>>> gdbserver.
>>>
>>> This patch moves the shareable code/data structures to
>>> common/target-common.* and makes both GDB and gdbserver target files
>>> reference the header common/target-common.h.
>>>
>>> Makefiles and other files have been adjusted accordingly.
>>
>> I'd very much prefer avoiding "common" in file names, instead
>> naming the files for what they contain, not for the fact that they're
>> "common" to two programs (gdb, gdbserver) presently.  I think of it
>> this way -- when we finally end up with only one backend (or one
>> backend using a foo-common.c file), I'd rather avoid
>> renaming these files to something else, because they're no longer
>> "common".  Or, yet IOW, think of common/ as a library.  Can you
>> imagine if all libraries in a distro named their implementation
>> files "foo-common.c" ?  Because that's what should happen given
>> they're used by lots of programs, right?  :-)  The direction I prefer
>> is, when moving things to common/ we take the opportunity to split them
>> into smaller, more atomic, leaner units.  E.g., that's how we ended up
>> with ptid.h/ptid.c, instead of inferior-common.h (or some such).
> 
> It makes sense to me, though i think we may have to think about creating 
> directories of subsystems instead of laying files the old way in a flat 
> directory structure.

Agreed.

My bare-minimal initial idea was to end up with the target backend stuff
in its own dir:

gdb/common/  (utils.c, etc., host independent bits.)
gdb/target/  (the backends)

(Given git is smart enough to understand moves automatically, I no
longer worry about losing cvs history when doing file moves these days.)

> We still have a handful of files in common, so maybe it is a good 
> starting point for that.

> 
>> If the file is just a dumping ground of misc things, then let's at
>> least call it that.  Say, target-misc.h or target-defs.h.
> 
> I'll declare them target-defs.h and target-misc.h/target-misc.c. How 
> does that sound?

No real objection, but is target-waitstatus.h/target-waitstatus.c too
fine-grained?

>> (I have absolutely nothing again John, but this shows how
>> "contributed by"/"written by" lines are a disservice to future
>> hackers, IMO.  Lot's of code here that others wrote.)
>>
> 
> Not wanting to offend John, but should i just remove this line in the 
> upcoming patch?

IMO, yes.  This new file holds bits that are newer than the
original target vector.

>>> +/* Interface between the debugger and target environments, including files
>>> +   and processes, shared between GDB and gdbserver.

The leading comment should be adjusted as well.  This file does
not really hold the target interface definition.

-- 
Pedro Alves