This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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



thanks for the quick response Kaveh.


On Fri, 9 Jun 2006, Kaveh R. Ghazi wrote:


> void print_string_array (const char *array_name, > const char *string, ...) __attribute__ > ((__sentinel__)); > > print_string_array ("empty_array", NULL); /* gcc warns, but shouldn't */ > > The only way out for keeping the sentinel attribute and avoiding the > warning is using > > static void print_string_array (const char *array_name, ...) > __attribute__ ((__sentinel__));

I think you could maintain typesafety and silence the warning by
keeping the more specific prototype and adding an extra NULL, e.g.:

print_string_array ("empty_array", NULL, NULL);

Doesn't seem elegant, but it does the job.

this is an option for a limited set of callers, yes.


> By the way, there is already an existing gcc bug, which is about the
> same thing (NULL passed within named args), but wants to have it the
> way it works now:
>
>   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21911

Correct, the feature as I envisioned it expected the sentinel to
appear in the variable arguments only.  This PR reflects when I found
out it didn't do that and got around to fixing it.  Note the "buggy"
behavior wasn't exactly what you wanted either because GCC got fooled
by a sentinel in *any* of the named arguments, not just the last one.


> so if it gets changed, then gcc might need to support both > - NULL termination within the last named parameter allowed > - NULL termination only allowed within varargs parameters (like it is > now)

I'm not against this enhancement, but you need to specify a syntax
that allows the old behavior but also allows doing it your way.

Hmm, perhaps we could check for attribute "nonnull" on the last named
argument, if it exists then that can't be the sentinel, if it's not
there then it does what you want.  This is not completely backwards
compatible because anyone wanting the existing behavior has to add the
attribute nonnull.  But there's precedent for this when attribute
printf split out whether the format specifier could be null...

We could also create a new attribute name for the new behavior.  This
would preserve backwards compatibility.  I like this idea better.

i agree here. as far as the majority of the GLib and Gtk+ APIs are concerned, we don't really need the flexibility of the sentinel attribute but rather a compiler check on whether the last argument used in a function call is NULL or 0 (regardless of whether it's the last named arg or already part of the varargs list). that's also why the actual sentinel wrapper in GLib looks like this:

#define G_GNUC_NULL_TERMINATED __attribute__((__sentinel__))

so, if i was to make a call on this issue, i'd either introduce
__attribute__((__null_terminated__)) with the described semantics,
or have __attribute__((__sentinel__(-1))) work essentially like
__attribute__((__sentinel__(0))) while also accepting 0 in the position
of the last named argument.

Next you need to recruit someone to implement this enhancement, or
submit a patch. :-) Although given that you can silence the warning by
adding an extra NULL at the call site, I'm not sure it's worth it.

i would say this is definitely worth it, because the issue also shows up in
other code that is widely used:
gpointer g_object_new (GType object_type,
const gchar *first_property_name,
...);
that's for instance a function which is called in many projects. putting the burden on the caller is clearly the wrong trade off here.


so please take this as a vote for the worthiness of a fix ;)

		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu

--- ciaoTJ


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]