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

[Kris Warkentin <kewarken at qnx dot com>]

Mark Kettenis wrote:

>   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.
>  
Question.  I use the nto_xxx_yyy functions _everywhere_ in our code.  
Would it be acceptable if I changed the define to be like target.h as in:

#define nto_xxx_yyy(args) (*current_nto_target.xxx_yyy) (args)

Then the only time I would do the full current_nto_target.xxx_yyy is in 
cases like below where it needs to be obvious that I'm testing or 
setting a global variable.  Its just that there are so many instances of 
these macros in our target support that the syntactic sugar is very 
attractive for code readability.

>   +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.
No problem.  Thanks.

Kris