Daniel Jacobowitz wrote:

> On Tue, Sep 06, 2005 at 12:09:37PM -0400, Kris Warkentin wrote:
> > Hi,
> >
> > I'm trying to break things up into smaller patches.  The actual remote
> > support is the biggest patch.  These headers are nice and bite sized 
> and
> > most of our other targets are fairly discrete as well.
> >
> > The file dsmsgs.h describes the structures used by the remote protocol
> > (pdebug).  This is identical to what the pdebug server uses on the
> > target side.
> >
> > The debug.h is a subset of our system header of the same name (and some
> > others) and describes the structures returned by various debug 
> interface
> > system commands.  Basically allows the same stuff as in our native
> > 'nto-procfs.c' to work in the absence of system headers.
> >
> > ChangeLog:
> >
> > 2005-09-06  Kris Warkentin  <kewarken@qnx.com>
> >
> >    * nto-share/debug.h: New file.
> >    * nto-share/dsmsgs.h: New file.
> >
> > Look okay?
>
> No...
>
> First of all, what purpose do you intend for these interfaces to serve
> in GDB?  Are they going to be used only by new code to implement the
> pdebug protocol, or shared between combined native/remote code?
>

Hi Daniel.  Thanks for looking at this for me.  I had split the headers 
to make the patch simpler but perhaps that wasn't the best idea.  I've 
attached the remote-nto.c file that uses these headers to this email.  
The headers are used by remote-nto.c to implement the QNX pdebug 
protocol which you get with 'target qnx <device>|<host:port>'

> Assuming remote only, then this:
> > /* __DEBUG_H_INCLUDED is Neutrino's native debug.h header.  We don't 
> want
> >    these duplicate definitions if we're compiling natively and have 
> already
> >    included it.  */
> > #ifndef __DEBUG_H_INCLUDED
> > #define __DEBUG_H_INCLUDED
>
> is pretty suspect - just give them different names from the system
> version, or avoid including the system header, preferably the latter.
>

I could re-name the structures reasonably easily.  I'm not so sure about 
avoiding the system header since the native support is using all of our 
kernel interfaces to issue devctls to the procfs debug interface.

> And basically everything else in the file is broken.  You can't do
> anything even vaguely complicated involving cross-debugging using host
> system types.  You're relying on the host's alignment and padding
> rules, and system types like clock_t.
>

Well, I don't know about "can't".  We support debugging of i386, sh, 
mips, arm and powerpc from Windows, Linux, Solaris and self-hosted 
Neutrino and have for many years using this code.  I definitely agree 
with you that the gdb remote protocol is far more portable and that 
passing structures around isn't the safest way to do things.  This is, 
however, the way pdebug has always been defined and provided everyone is 
using gcc, everything works fine.

Basically, I'm acknowledging the limitations of this code and hoping 
that, with certain changes and cleanup, you might consider accepting it 
anyway.  Several points:  1) It's a tried and tested solution that has 
been in the field supported by us for more than half a decade.  2) It 
doesn't actually have anything to do with any other gdb mechanisms.  
Completely self contained.  If we broke anything, it would only be our 
own stuff.  3) In order to make it completely portable, I'd most likely 
have to avoid passing structures completely and totally re-write the 
entire protocol and server.

> And whatever this does:
>
> > #ifdef __QNX__
> > __BEGIN_DECLS
> > #include <_pack64.h>
> > #endif
>
> is similarly not OK.  If it's intended to work cross, then it should
> work from everywhere, not just from x86 and QNX.
>

That's easy enough to fix.

> I think I need a mile-high explanation of what you're trying to do,
> first.  By the way, is pdebug publically documented?
>

I hope I've more or less explained it above.  I want to submit a 
self-contained patch for our remote protocol that, while not the most 
portable of solutions, will not interfere with anything else.  As far as 
documentation goes, the descriptions within the headers and source files 
are just about it.  Gdb blocks on the remote which eventually sends back 
a reply stating the type of event that occurred after which gdb gathers 
whichever information it needs and issues commands back to the target.  
There's a separate channel for I/O.  Fairly simple all told.

cheers,

Kris