Re: [RFC] Use target vector inheritance for GNU/Linux

[Mark Kettenis <kettenis at gnu dot org>]

   Date: Mon, 13 Dec 2004 15:41:56 -0500
   From: Andrew Cagney <cagney@gnu.org>

   Mark Kettenis wrote:

   >    Can you rename saved_xfer_partial to super_xfer_partial_hack and add a 
   >    FIXME.  It should be calling super.xfer_partial but that's not available :-(
   > 
   > Can you explain what you're hinting at here Andrew?  What makes this
   > specific saved_xfer_partial so different from the other saved_xxx
   > instances that the patch introduces?

   Nothing?  Changing those to be consistent with this would be a logical 
   next step.

I suspect I don't see the big picture like you do, and I don't see why
you'd want a super.xfer_partial_hack.  This construction isn't a hack
at all; it explicitly shows how the method is inherited.  However,
super_xfer_partial is perhaps a better name than saved_xfer_partial.

   >    +#ifndef FETCH_INFERIOR_REGISTERS
   >    +
   >    +/* Fetch register REGNUM from the inferior.  */
   >    +
   >    +static void
   >    +fetch_register (int regnum)
   >    +{
   > 
   >    Why is this wrapped in in an #ifdef?
   > 
   > Some of the Linux target still need the crufty old code to read
   > registers using PTRACE_PEEKUSR.  The new inf-ptrace.c doesn't provide
   > that functionality, so I guess Daniel inlined that bit of code here.
   > This is related to the FIXME below, and of course only temporary.

   So a fixme or other comment needs to be added at the point of this macro?

Wouldn't hurt, although that FIXME below is already there.

   >    +/* Create a generic GNU/Linux target vector.  If T is non-NULL, base
   >    +   the new target vector on it.  */
   >    +
   >    +struct target_ops *
   >    +linux_target (struct target_ops *t)
   > 
   >    Can this be renamed to inf_linux_target (to be consistent with the other 
   >    inf_*_target() methods?
   > 
   > Apparently I don't agre with this since I already introduced
   > i386bsd_target and sparc_target; linux_target is consistent with that.

   You also added inf_ttrace_target.

Yup, since that one lives in inf-ttrace.c.  The inf_ttrace_ prefix is
consistent with the naming of the other functions in the file, just as
i386bsd_target and sparc_target are consistent with the files they're
in.

   > No it isn't.  At a very low level, all Linux ports are slightly
   > different.  Most ports will need to adjust the generic ptrace target
   > before it can be inherited by the generic Linux target.  In fact I
   > think that when the FETCH_INFERIOR_REGISTERS issue above is sorted
   > out, you'll see that *all* Linux ports will need to do this trick of
   > adjusting the ptrace target before passing it to linux_target().
   > 
   > (And yes, I'm fairly certain the method is still needed.  While the
   > problem may have been fixed in recent kernels, there are many older
   > Linux kernels out there.)

   You're on the right track, however the inheritance structure that the C 
   code is trying to mimic is:

   i386LinuxInferior IS-A LinuxInferior IS-A PtraceInferior

Ah but that's not how the Linux-interfaces work (unless we drop
support for multi-threading).  LinuxInferior needs platform-dependent
functionality that PtraceInferior doesn't (and shouldn't) provide.  

   with the i386LinuxInferior.resume method overriding LinuxInferior.resume 
   (which overrid PtraceInferior.resume); and not:

But having i386LinuxInferior override LinuxInferior.resume really
sucks because then it too has to do this insane dance of fiddling with
LWPs because of the braindamaged Linux threading model.

   LinuxInferior IS-A i386LinuxInferior IS-A PtraceInferior

This reflects reality better, although it might even be:

i386LinuxInferior IS-A LinuxInferior IS-A i386LinuxPtraceInferior
  IS-A PtraceInferior

But that breaks some OO rules of course.

   That the current inferior code doesn't facilitate this is a recognized 
   problem, but not one that we should get hung-up over.  Hence my 
   suggestion of deprecated_set_super_linux_resume as a workaround until 
   that is fixed.

It's not a problem with the current inferior code.  There may be some
issues with the inferior code (like the ptrace_ops_hack), but this is
a fundamental problem with the Linux threads debugging interface.
Perhaps there simply shouldn't be a LinuxInferior:

i386LinuxInferior IS-A PtraceInferior

where we'de have some Linux-specific helper functions that can be used
to implement i386LinuxInferior.

Mark