Patch: Problem with type safety and the "sentinel" attribute

[Stefan Westerfeld]

   Hi!

On Thu, Jun 15, 2006 at 03:34:06PM +0200, Tim Janik wrote:
> On Thu, 15 Jun 2006, Stefan Westerfeld wrote:
> >There has been some discussion with the same subject on the gcc list,
> >but I am moving this to gcc-patches now, since I've got a patch now,
> >which you'll find at the end of the mail.
> >
> [...]
> 
> >Index: gcc/doc/extend.texi
> >===================================================================
> >--- gcc/doc/extend.texi	(revision 114677)
> >+++ gcc/doc/extend.texi	(working copy)
> >@@ -1561,8 +1561,8 @@
> >attribute specification inside double parentheses.  The following
> >attributes are currently defined for functions on all targets:
> >@code{noreturn}, @code{returns_twice}, @code{noinline}, 
> >@code{always_inline},
> >-@code{flatten}, @code{pure}, @code{const}, @code{nothrow}, 
> >@code{sentinel},
> >-@code{format}, @code{format_arg}, @code{no_instrument_function},
> >+@code{flatten}, @code{pure}, @code{const}, @code{nothrow}, 
> >@code{null_terminated},
> >+@code{sentinel}, @code{format}, @code{format_arg}, 
> >@code{no_instrument_function},
> >@code{section}, @code{constructor}, @code{destructor}, @code{used},
> >@code{unused}, @code{deprecated}, @code{weak}, @code{malloc},
> >@code{alias}, @code{warn_unused_result}, @code{nonnull}
> >@@ -2155,6 +2155,42 @@
> >take function pointer arguments.  The @code{nothrow} attribute is not
> >implemented in GCC versions earlier than 3.3.
> >
> >+@item null_terminated
> >+@cindex @code{null_terminated} function attribute
> >+This function attribute ensures that the last parameter in a function
> >+call is an explicit @code{NULL}. The attribute is only valid on variadic
> >+functions. It is very similar to the @code{sentinel} attribute. However
> >+it allows calls to the function without any variadic arguments, so that
> >+the @code{NULL} termination occurs within the last named parameter.
> >+
> >+@smallexample
> >+static void print_string_array (const char *array_name,
> >+                                const char *string, ...) __attribute__ 
> >((null_terminated));
> >+
> >+void
> >+foo()
> >+@{
> >+  /* invalid with null_terminated attribute and invalid with sentinel 
> >attribute */
> >+  print_string_array ("bad_array", "bad1", "bad2");
> >+
> >+  /* valid with null_terminated attribute, but invalid with sentinel 
> >attribute */
> >+  print_string_array ("empty_array", NULL);
> >+
> >+  /* valid with null_terminated attribute and valid with sentinel 
> >attribute */
> >+  print_string_array ("foo_array", "string1", "string2", "string3", NULL);
> >+@}
> >+@end smallexample
> >+
> >+A valid @code{NULL} in this context is defined as zero with any pointer
> >+type.  If your system defines the @code{NULL} macro with an integer type
> >+then you need to add an explicit cast.  GCC replaces @code{stddef.h}
> >+with a copy that redefines NULL appropriately.
> 
> why does 0 have to be a pointer type here?
> Gtk+/GObject could benefit from 0 as well, e.g. we ahve APIs that take
> (type,value) pairs in argument lists, such as:
>   gtk_tree_store_set (tree_store, iter,
>                       G_TYPE_STRING, "foo",
>                       G_TYPE_INT, 17,
>                       0);
> 
> to clarify, the way i suggested null_terminated in the original thread,
> it only differs from sentinel(0) in accepting 0 or NULL in the last named
> argument. it doesn't require the terminating 0 to be a pointer type.

But then, given this prototype:

static void print_string_array (const char *array_name,
                                const char *string, ...) __attribute__ ((null_terminated));

with your proposal, the following code is valid:
  
  print_string_array ("foo", "foo1", "foo2", 0);

Which will lead to segfaults on AMD64 (since sizeof (0) != sizeof
(NULL)). More so, the problem which I wanted to solve by adding the
attribute (that sometimes NULL in C++ can be defined in a wrong way)
will no longer get catched by gcc.

That is:

/* NULL in C++ as found in linux/stddef.h on some systems: */
#undef NULL
#define NULL 0

  print_string_array ("foo", "foo1", "foo2", NULL);

will also compile and segfault on AMD64, although I my goal is to have
the null_terminated attribute catch this case. That doesn't rule out
that you may wish for other features - maybe they can even be supported
by the same attribute, with extra syntax.

But still it should be possible to reliably detect whether an argument
list is properly (segfault free) NULL terminated or not with this
attribute, otherwise its not very useful.

   Cu... Stefan
-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan