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

[Mark Kettenis <kettenis@gnu.org>]

   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