Re: Revised: [patch] general updates and improvements to QNX NTO support

Mark Kettenis
2004-12-08T22:20:06Z

Date: Mon, 06 Dec 2004 10:30:46 -0500
From: Kris Warkentin <kewarken@qnx.com>

Based on Mark's comments, I've refactored this patch to provide a
cleaner interface to nto_tdep.c. I created an nto_set_target function
so that the targets can initialize their own nto_target_ops and set the
target properly rather than initializing current_nto_target.

Thanks. That looks good! I still think you should avoid the

#define nto_xxx_yyy (current_nto_target.xxx_yyy)

defines. They're hiding the fact that you're using a global variable.
Consider a patch that uses current_nto_target.xxx_yyy directly
pre-approved. I've got one additional nit, please see below.

I thought about completely hiding the implementation by turning all the
macros into functions which just call the current target functions but
I'm not sure if it's worth it. Specifically I'm thinking about cases
where other modules need access to members of current_nto_target. For
instance, the remote and procfs modules need to set various flags and
hooks so if I were to hide current_nto_target, I'd need setter/getter
functions. Certainly that is the cleaner 'OO' approach. If it needs
changing later, I might also store a list of targets in nto-tdep.c. It
doesn't seem like a high priority right now though since we probably
won't be multi-arching our gdb any time soon.

This can be done when the need arises.

+enum gdb_osabi
+nto_elf_osabi_sniffer (bfd *abfd)
+{
+ if (nto_is_nto_target)
+ {
+ return nto_is_nto_target (abfd);
+ }
+ return GDB_OSABI_UNKNOWN;
+}
+

Could you remove the redundant braces here? With that change, this is
OK.

Thanks,

Mark