> Date: Sat, 2 Apr 2011 11:13:05 +0000
> From: Miod Vallat <[email protected]>
>
> The -Wsentinel warning is supposed to complain when the last argument in
> a call to a function which has __attribute__((sentinel)) is not a NULL
> pointer.
>
> However, the current implementation of the check in gcc 3 and gcc 4 will
> warn if the last argument is not explicitely declared as a pointer type,
> i.e.
> execl(foo, bar, (const char *)NULL)
> will not warn, but
> execl(foo, bar, NULL)
> will cause a false warning, because NULL is defined as 0L for non-C++
> code.
>
> The following diff relaxes the check to let it accept either a pointer
> or an integer type of the same width as a pointer (i.e. on a 64 bit
> platform, using 0L will compile silently, but using 0 will).
>
> Ok? (note that this has only been tested on gcc 4, the gcc 3 version is
> similar but has not even been checked to compile yet)
Not really ok. Given the way the ISO C standard is formulated, not
casting NULL in this case is a genuine bug.
One can argue though, that defining NULL in such a way that it is not
usable as a sentinel value would be really really silly, and that it
is not our job to catch missing casts like this. In that case though,
I'd propose that we change our definition of NULL to
#define NULL ((void *)0)
instead of modifying our compilers.
> Index: gnu/gcc/gcc/c-common.c
> ===================================================================
> RCS file: /cvs/src/gnu/gcc/gcc/c-common.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 c-common.c
> --- gnu/gcc/gcc/c-common.c 12 May 2010 13:35:20 -0000 1.3
> +++ gnu/gcc/gcc/c-common.c 2 Apr 2011 11:04:49 -0000
> @@ -5498,15 +5498,17 @@ check_function_sentinel (tree attrs, tre
> }
>
> /* Validate the sentinel. */
> - if ((!POINTER_TYPE_P (TREE_TYPE (TREE_VALUE (sentinel)))
> - || !integer_zerop (TREE_VALUE (sentinel)))
> + sentinel = TREE_VALUE (sentinel);
> + if ((!(POINTER_TYPE_P (TREE_TYPE (sentinel))
> + || (TREE_CODE (sentinel) == INTEGER_CST
> + && TYPE_PRECISION (TREE_TYPE (sentinel)) == POINTER_SIZE))
> + || !integer_zerop (sentinel))
> /* Although __null (in C++) is only an integer we allow it
> nevertheless, as we are guaranteed that it's exactly
> as wide as a pointer, and we don't want to force
> users to cast the NULL they have written there.
> We warn with -Wstrict-null-sentinel, though. */
> - && (warn_strict_null_sentinel
> - || null_node != TREE_VALUE (sentinel)))
> + && (warn_strict_null_sentinel || null_node != sentinel))
> warning (OPT_Wformat, "missing sentinel in function call");
> }
> }
> Index: gnu/usr.bin/gcc/gcc/c-common.c
> ===================================================================
> RCS file: /cvs/src/gnu/usr.bin/gcc/gcc/c-common.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 c-common.c
> --- gnu/usr.bin/gcc/gcc/c-common.c 16 Oct 2009 12:22:07 -0000 1.6
> +++ gnu/usr.bin/gcc/gcc/c-common.c 2 Apr 2011 11:04:50 -0000
> @@ -6502,8 +6502,11 @@ check_function_sentinel (attrs, params)
> }
>
> /* Validate the sentinel. */
> - if (!POINTER_TYPE_P (TREE_TYPE (TREE_VALUE (sentinel)))
> - || !integer_zerop (TREE_VALUE (sentinel)))
> + sentinel = TREE_VALUE (sentinel);
> + if (!(POINTER_TYPE_P (TREE_TYPE (sentinel))
> + || (TREE_CODE (sentinel) == INTEGER_CST
> + && TYPE_PRECISION (TREE_TYPE (sentinel)) == POINTER_SIZE))
> + || !integer_zerop (sentinel))
> warning ("missing sentinel in function call");
> }
> }
