This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: patch to add QNX NTO i386 support


> Suggest separating the GDB stuff out (native, target, remote) and using
> separate e-mail threads to discuss each.

I was going to but it's easier this way.  The native nto-procfs.c makes use
of some of the code in remote-qnx.c and remote-qnx-<target>.c (we still have
four more targets.)  If you really feel it's necessary I could do the work
but I had started on it and concluded it would either lead to a lot of
duplicated code or an explosion of files.

> > gdb/ChangeLog entry:
> >
> > 2003-03-02    Kris Warkentin    kewarken@qnx.com
> >
> >     * config/i386/i386nto.mt: New file
>
> The MH_CFLAGS and XM_FILE flags should not be needed.  Instead
> gdb/configure should be able to handle this.

Noted.

> >     * config/i386/nm-nto.h: New file
>
> The file nm-nto.h should not be needed.  Instead define it's only macro
>   local to remote-nto.c.  (Disclaimer, you're breaking new ground with
> this one.  Some existing targets  don't have xm-*.h files, but I think
> you're first with the no-*.h file).

Not sure what you mean by no-*.h but I see what you mean about some of the
short files.

> >     * config/i386/nto.mh: New file
>
> Yes, you need this, you've a native support.

Cool.  I got ONE thing right. ;-)

> >     * config/i386/tm-i386nto.h: New file
>
> The file tm-i386nto.h should not be needed.  Instead, gdbarch handles
> architecture specific issues.  The only exception is with shared
> libraries (as there is a bit of this that isn't yet multi-arched).  If
> nto has architecture specific features then create an i386-nto-tdep.c or
> nto-tdep.c file (typically it ends up containing the sigtramp code).

Okay.  I'll look into this.

> >     * config/i386/xm-nto.h: New file
>
> The file xm-nto.h should not be needed.  gdb/configure should be able to
> handle all host specific problems.

Okay.

> >     * config/tm-qnxnto.h: New file
> >     * configure.host: add gdb_host=nto
> >     * configure.tgt: add gdb_target=i386nto
> >     * nto-procfs.c: New file
>
> Once the target stuff is sorted, please re-submit nto-procfs.c as a
> separate patch.
>
> >     * nto-share/debug.h: New file
> >     * nto-share/dsmsgs.h: New file
>
> >     * remote-nto-i386.c: New file
> >     * remote-nto.c: New file
>
> Can you expand on how these relate to each other?

Do you want details in the ChangeLog entry other than 'new file'?  I wasn't
sure.
FYI:

debug.h has all the procfs stuff needed for all our targets.
dsmsgs.h has our remote pdebug debugging protocol definition.
remote-nto.c has all generic code for remote debugging plus code shared by
local and remote.
remote-nto-<cpu>.c is any specific code for a particular cpu.  This can also
be shared by local and remote.

> >     * ser-ntopty.c: New file
>
> Hmm, is this specific to nto?  Also, glancing through the code, how
> different is this to the existing serial code.  I'm wondering if the
> file exists due to local fixes and not because it is needed.

Not 100 percent sure.  That's one of the files that I inherited.  It's
possible that it's not needed anymore so I'll have to look a little deeper
and get back to you.

> Regarding coding standards.
>
> Look through http://sources.redhat.com/gdb/current/ari/.
> Anything listed under `Critical', `Code', or `Fixed' should be
> addressed, also glance through `info'.  I immediatly noticed `PARAMS()'
> (GDB uses ISO C) `//' (C, not C++) and `extern' in C files (always bad).

Ah...lint. ;-)

> All files need a proper (C) notice at the top.  The year should include
> `2003' since this is the year that the FSF will first include it.

Okay.

> Run all the new files through ./gdb_indent.sh.

Did that.

> Audit all `#ifdef ...' many of those macro's have been deleted (I
> noticed TARGET_BYTE_ORDER_SELECTABLE'.

Okay.

> Many of the functions have `^qnx_' prefixes.  Should they be given
> `^nto_' prefixes?

Yeah.  All the original code and filenames used 'qnx' and I partially
refactored to 'nto' since it seemed more appropriate to use the OS rather
than the company.  I suppose it wouldn't hurt to do the rest of it.

Thanks for all the feedback.  You've certainly given me some more work to
keep me occupied.  You're probably right about some of the code being
redundant.  Much of this stuff was brought forward from gdb 4.17 so there
may be many more redundancies.  (you should have seen how much we had ripped
out already ;-)

cheers,

Kris


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]