From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2085 invoked by alias); 13 Dec 2004 20:44:31 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 1926 invoked from network); 13 Dec 2004 20:44:16 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 13 Dec 2004 20:44:16 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id iBDKiGw5017342 for ; Mon, 13 Dec 2004 15:44:16 -0500 Received: from localhost.redhat.com (vpn50-64.rdu.redhat.com [172.16.50.64]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id iBDKi6r21059; Mon, 13 Dec 2004 15:44:06 -0500 Received: from [127.0.0.1] (localhost.localdomain [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id BBF6C3EF9; Mon, 13 Dec 2004 15:41:58 -0500 (EST) Message-ID: <41BDFE94.9060401@gnu.org> Date: Mon, 13 Dec 2004 20:45:00 -0000 From: Andrew Cagney User-Agent: Mozilla Thunderbird 0.8 (X11/20041020) MIME-Version: 1.0 To: Mark Kettenis Cc: drow@false.org, gdb-patches@sources.redhat.com Subject: Re: [RFC] Use target vector inheritance for GNU/Linux References: <20041205184549.GA19814@nevyn.them.org> <41BC8521.9060107@gnu.org> <200412122107.iBCL7KEi011314@elgar.sibelius.xs4all.nl> In-Reply-To: <200412122107.iBCL7KEi011314@elgar.sibelius.xs4all.nl> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-12/txt/msg00365.txt.bz2 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. > +#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? > +/* 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. > > A new function, linux_target, is added to linux-nat.c. Then any GNU/Linux > > target can call it, and pass the result to add_target - after specializing > > whatever methods it needs to. Sometimes it's necessary to specialize a > > method between inf_ptrace_target and linux_target, so it accepts an optional > > argument. This wouldn't be necessary if all target methods took a > > target_ops parameter, so they could call the overridden method. > > As in this? > > > +void > > +_initialize_i386_linux_nat (void) > > +{ > > + struct target_ops *t = inf_ptrace_target (); > > + > > + /* Override the default ptrace resume method. */ > > + t->to_resume = i386_linux_resume; > > + > > + /* Fill in the generic GNU/Linux methods. */ > > + t = linux_target (t); > > which is violating the inheritance structure. Can you instead add a > one-of method (deprecated_set_super_linux_resume?) to handle this case? > We can then see about fixing the problem (I'm left wondering if that > method is still needed). > > 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 with the i386LinuxInferior.resume method overriding LinuxInferior.resume (which overrid PtraceInferior.resume); and not: LinuxInferior IS-A i386LinuxInferior IS-A PtraceInferior 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. Andrew