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

Pedro Alves
2013-08-05T10:44:03Z

On 08/02/2013 09:48 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> "target" is an overloaded word in GDB-speak. My idea for this new
> Pedro> directory, would be for it to hold the native target backend bits.
> Pedro> But "target" could also suggest that corelow.c, file.c, remote.c, etc.
> Pedro> should be put in this directory, while I don't think they should.
>
> I've been thinking about this a bit since the discussion yesterday.
>
> I think I'm generally in favor of using the names Luis already has.
>
> The basic reason I have is that I think that, by and large, gdb's module
> boundaries make sense. I may quibble with some exact lines that have
> been drawn (and certainly I dislike the insides of some modules), but by
> and large the modules, at least as I understand the breakdown, have
> proven resilient.
>
> I agree that "target" is not the best possible name de novo. However,
> it is the nature of language to overload words with meanings -- the
> norm, not the exception -- and furthermore "target" is the name
> historically chosen inside gdb to represent the connection between the
> core (+ CLI) and the back end.

I've read your email several times over, and I sense that we're
talking past each other. Myself, I'm used to the use of "target",
in "struct target_ops" & friends, and I have no desire to push
into a change here. "back end" is also a little vague, in that
the gdbarch bits could also be seen as a backend. When trying to
think of a more precise term, I end up thinking of "target"
representing the connection between the core and a system's debug
API (though that has it's own shortfalls too). So "target" being
vague is good. :-)

> You had a few specific issues, which I've quoted & will address below.
>
> Pedro> Sounds like a better name for this native target backend directory
> Pedro> should be invented. GDB uses *-nat.c naming for most of
> Pedro> these files, while GDBserver uses *-low.c.
>
> I think it's fine to use "nat" in the same way that gdb does now.
>
> Looking at the current patch, though, I don't see anything "nat" in
> there.

Sure. But this thread is coming as preparatory work for the
ptrace-sharing patch, and the gdbserver fork support work, and if you
look at http://sourceware.org/ml/gdb-patches/2013-07/msg00840.html
you'll see that the original "target/" directory was thrown around
as place for holding the native backends.
If we don't use "target/" for the native backends, then, yes,
a "target/" directory for target_ops things would make better
sense. (I'm not excluding a "target/nat/linux/" subdirectory
scheme, though, to me, that's less a concern than 'target/ for all'
vs 'target/ + nat/'.)

> The ptrace options discovery bits could go there, but that
> wasn't addressed in this thread IIRC.

So what I'm trying to point out, is that the new files in
the patch in these threads are not really "native backend"
files, so they best not go in the "native backend" files
directory.
IOW, after this, and
http://sourceware.org/ml/gdb-patches/2013-07/msg00788.html
and continuing that direction, I don't think we would want
to end up with:

target/resume.h
target/waitstatus.[c|h]
target/wait.h
target/i386-nat.c
target/linux-nat.c
target/linux-ptrace.c
target/linux-waitpid.c
etc.

>
> Pedro> These new target-resume.h, target-wait.h, target-waitstatus.h,
> Pedro> target-waitstatus.c files might be looked at as actually something
> Pedro> else. This is code defining the interface between GDB core and
> Pedro> target_ops, and as such is used by all sort of targets on the
> Pedro> GDB side (remote.c, etc.). I'm not sure they should go in the same
> Pedro> directory as the *-nat.c, etc. files.
>
> These seem like classic "target" bits to me.

Yep. So, if we move the classic "target" bits to a "target/"
module directory, and put the native bits in their own dir, we
have:

target/resume.h
target/waitstatus.[c|h]
target/wait.h
nat/i386-nat.c
nat/linux-nat.c
nat/linux-ptrace.c
nat/linux-waitpid.c
etc.

Is this what you're thinking of? _This_, I'm fine with.
No mix of native bits with generic "target" bits, which was
my main worry.

It's actually very similar to something else I suggested on IRC,
but forgot to put in email form: "IMO, the interfaces themselves would be
in an include dir. e.g., gdb/include/target-waitstatus.h or some such,
and then we'd have gdb/nat/linux-nat.c, etc."

What else goes in "target/" ? remote.c, corelow.c, etc.?
Do we move things into subdirectories beneath it too, for better
submodule partitioning? I didn't want to suggest starting a
mass move, that's easy to overdo. (That was the point at which I
suggested that someone thinks this through, and comes up with an
initial design/guide of what things will look like in the end, so
that we can then discuss and hash it out.)

--
Pedro Alves