Re: `sentinel' gcc-3.x/OpenBSD compat. [Re: [patch 1/3] Make obconcat use stdarg]

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Sat, 1 May 2010 00:18:39 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Fri, 30 Apr 2010 21:08:48 +0200, Mark Kettenis wrote:
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > 
> > > __attribute__ ((sentinel)) availability for gcc >= 4.0 I have copied from
> > > <glib-2.0/glib/gmacros.h>.  It roughly matches the GCC ChangeLog dates.
> > 
> > The OpenBSD system compile, which is based on GCC 3.3.5, already has
> > the sentinel attribute.
> 
> FSF GCC 3.3.5 does not support this attribute:
> 	void f (int i, ...) __attribute__ ((__sentinel__));
> 	sentinel.c:1: warning: `__sentinel__' attribute directive ignored
> 	(+ there are no traces of any `sentinel' in the gcc-3.3.5 sources)
> 
> OpenBSD-4.6-amd64 from md5sum
> 	28edaa65b16bd5381ebf645d27039a94  openbsd-install46.iso
> does not boot for me on Fedora 12 x86_64 KVM using
> 	kernel-2.6.32.11-99.fc12.x86_64
> 	qemu-system-x86-0.11.0-13.fc12.x86_64
> as it hangs on "setting tty flags"; I have not found any OpenBSD Live image.

OpenBSD runs on real hardware so we blame this one on KVM ;)

> Moreover I do not consider relevant to support this attribute on so obsolete
> compiler.  Not using the attribute has no effect on users building GDB.
> This attribute is useful only for the GDB developers.

Fair enough; this feature is nice to have but in no way essential.

> > I'm afraid you'll need an explicit cast for the NULLs used as the
> > sentinel value, otherwise platforms that
> > 
> >   #define NULL 0L
> > 
> > will generate warnings like:
> > 
> >   sentinel.c: In function 'foo':
> >   sentinel.c:8: warning: missing sentinel in function call
> > 
> > and with -Werror, that's not a good :(.
> 
> Which platforms?  Could you provide a login there?  As NULL is commonly
> defined using (such as in <dbus-1.0/dbus/dbus-macros.h>):
> 
> #ifndef NULL
> #  ifdef __cplusplus
> #    define NULL        (0L)
> #  else /* !__cplusplus */
> #    define NULL        ((void*) 0)
> #  endif /* !__cplusplus */
> #endif
> 
> forcing NULL to be ((void *) 0) may make GDB C++ compatibility problematic
> (I do not understand reasons for the conditional above, though).

Beats me why the dbus developers think they have to provide their own
definition of NULL when one is readily available by including <stddef.h>, even on freestanding environments.

I don't see why using (char *)NULL as sentinel would cause problems in
C++.  Anyway, GDB is written in ISO C90, not C++.

> OTOH a requirement of some special sentinel expression instead of
> simple `NULL' for obconcat would nullify its stdarg convenience.

Dunno, I don't think using (char *)NULL is much more complicated.  And
ultimately, given the way a null pointer constant is defined in ISO
C99, the cast is simply required for varargs functions.

> Therefore something could be placed to configure.ac for NULL
> redefinition depending on the test on the specific build system.

I think that would be a bad idea.