Re: [PATCH 3/7] Introduce nat/linux-namespaces.[ch]

[Gary Benson <gbenson at redhat dot com>]

Doug Evans wrote:
> On Thu, Apr 16, 2015 at 5:19 AM, Gary Benson <gbenson@redhat.com> wrote:
> > diff --git a/gdb/nat/linux-namespaces.c b/gdb/nat/linux-namespaces.c
> > new file mode 100644
> > index 0000000..f6fe863
> > --- /dev/null
> > +++ b/gdb/nat/linux-namespaces.c
> > @@ -0,0 +1,231 @@
[snip]
> > +#include "common-defs.h"
> > +#include "nat/linux-namespaces.h"
> > +#include "filestuff.h"
> > +#include <fcntl.h>
> > +#include <sys/syscall.h>
> > +#include <sys/stat.h>
> > +
> > +/* Handle systems with __NR_setns but no setns.  */
> > +#if defined(__NR_setns) && !defined(HAVE_SETNS)
> > +static int
> > +setns (int fd, int nstype)
> > +{
> > +  return syscall (__NR_setns, fd, nstype);
> > +}
> > +#define HAVE_SETNS 1
> > +#endif /* defined(__NR_setns) && !defined(HAVE_SETNS) */
> 
> Nit:
> I understand the goal here, but I think it would be better to do
> this slightly differently.
> The value of HAVE_SETNS is potentially different in this file than
> for the rest of gdb,
> and I generally think of config.h as being consistent across the app.
> I don't have a strong preference on how to change this, other than I
> think values defined in config.h should be left alone.

The value of HAVE_SETNS is potentially different in this file than for
the rest of gdb, but so is the existence of a setns function.  That's
why I did it that way.  I certainly didn't want to make our own setns
available to any other file, as really anything accessing namespaces
should go through this file.

> E.g., one could just just define another macro locally here.
> 
> #if defined(__NR_setns) || defined(HAVE_SETNS)
> #define REALLY_HAVE_SETNS 1
> #endif
> 
> #ifdef REALLY_HAVE_SETNS
> ... the rest of the file ...
> 
> or some such.

I could do that (I'd probably call it HAVE_LINUX_NAMESPACES).
I prefer the way I did it but I won't fight this.

Cheers,
Gary

-- 
http://gbenson.net/