Re: [PATCH] Refactor common/target-common into meaningful bits

[Pedro Alves <palves@redhat.com>]

On 08/01/2013 06:52 PM, Luis Machado wrote:
> On 08/01/2013 02:49 PM, Tom Tromey wrote:
>>>>>>> "Luis" == Luis Machado <lgustavo@codesourcery.com> writes:
>>
>> Luis> First, it seems like a good idea to stablish a  more meaningful
>> Luis> directory structure as well, so we are moving target-common.[c|h] from
>> Luis> the "common" dir to the new "target" dir. This new directory will hold
>> Luis> anything more backend-related. For now it contains only generic target
>> Luis> definitions and functions.
>>
>> I like it.
>>
>> Luis> I've broken target-common.[c|h] into the following:
>> Luis> - target-resume.h: Definition for resume_kind.
>> Luis> - target-waitstatus.[c|h]: Definitions and code for anything related
>> Luis> to waitstatus.
>> Luis> - target-wait.h: A tiny bit that does not seem to fit properly in the
>> Luis> waitstatus files, so it is left here.
>>
>> If I may be permitted to bikeshed just a bit longer...
>>
>> Right now the file is named "target/target-waitstatus.h" and the code
>> says:
>>
>> +#include "target-waitstatus.h"
>>
>>
>> I wonder whether you considered naming the file "target/waitstatus.h"
>> and having the code say:
> 
> I did and it does sound more intuitive, but i didn't want to turn too 
> many knobs at a time. If others are OK with this, i can make that change.

We had a discussion on IRC yesterday on this.  To sum it up:

"target" is an overloaded word in GDB-speak.  My idea for this new
directory, would be for it to hold the native target backend bits.
But "target" could also suggest that corelow.c, file.c, remote.c, etc.
should be put in this directory, while I don't think they should.

Sounds like a better name for this native target backend directory
should be invented.  GDB uses *-nat.c naming for most of
these files, while GDBserver uses *-low.c.

( "low" itself in GDBserver is also ambiguous -- e.g., linux-low.h
introduces the "struct linux_target_ops", and we call _that_ the
"low target" at places (seems its my own fault for introducing
that ambiguity...) ... )

So to me that suggests "nat", "native" or "low", in my order
of preference.

These new target-resume.h, target-wait.h, target-waitstatus.h,
target-waitstatus.c files might be looked at as actually something
else.  This is code defining the interface between GDB core and
target_ops, and as such is used by all sort of targets on the
GDB side (remote.c, etc.).  I'm not sure they should go in the same
directory as the *-nat.c, etc. files.  GDBserver's "struct target_ops"
is both smaller, and different at places from GDB's own "struct target_ops".
In a world where we fuse gdb's and gdbserver's target backends, it's
not clear to me at this point whether we'll end up with only one
"struct target_ops", or whether we'll still end up with two
different structures, with parts of GDBserver core implementing
GDB's own target_ops, and marshalling things to the lower GDBserver's
struct target_ops, while handling others behind GDB core's back.

I guess I'm saying, someone should sit, and think this
through, and come up with a design/guide of what things look like
in the end, before we start moving things around too much.  In
the mean time, I propose avoiding diverging from Luis' original
goal, and continuing what we had been doing, by
putting target-resume.h, target-wait.h, target-waitstatus.h,
target-waitstatus.c under common/.  Further moves can always
be done later.

WDYT?

-- 
Pedro Alves