This is the mail archive of the gcc-patches@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: Patch: New null_terminated attribute for C/C++


On 13 Oct 2002, Tom Tromey wrote:

> This patch adds support for a new `null_terminated' attribute, and
> -Wnull-termination option to gcc.
> 
> Some functions, like execl, require a trailing NULL argument.  Here
> the null argument must have pointer type, so in C++ this requires a
> cast.  It is easy to forget to leave off the terminating NULL.  Also,
> apparently there are cases where the user writes `0' instead of `NULL'
> in C code, leading to a problem when moving to a machine where
> sizeof(int)!=sizeof(void*).

And execle has an argument after the NULL argument, and maybe some
functions take multiple such lists.  You need a more general design (that
at least can handle execle).  If something is terminated by a single NULL,
it should also warn for multiple NULLs.  Find out whether Marc Espie did
any design for this sort of thing beyond the nonnull attribute support. It
wouldn't surprise me if some functions (whether standard or in user
software) expect a list of integers terminated by zero, or some other
termination rule: a survey of variadic functions in various software to
see what sort of termination rules occur would be useful to get an idea of
what features make sense here.

> Built on x86 Red Hat Linux 7.3.  I tested it using various
> permutations of a program like this:
> 
>     #include <stdio.h>
>     #include <stdarg.h>
> 
>     extern int doit (char *program, ...) __attribute__ ((null_terminated));
> 
>     int main (int argc, char **argv)
>     {
>       doit ("foo");
>     }
> 
> In particular I tested both C and C++, where the final argument is
> NULL, an ordinary char* value, or 0.  I also tested that you get a
> warning if the attribute is applied to something other than a function
> decl.

But you didn't include testcases for any of this.  They are required - see
codingconventions.html.  See the existing format checking testcases for
the thoroughness expected.

This should link more closely into the format checking support, as the
nonnull attribute does; this is another case of checking constraints on
function calls beyond standard type checking.  In particular, -Wformat
should enable it (like it enables -Wnonnull) and the recursion down
branches of arguments (here to check for any non-NULL branch) should
apply: the generic support is there, even though less likely to be of 
practical use in detecting bugs in this case.

> Ok to commit?

It's obviously inappropriate for mainline at this development stage, in
any case.

> +-Wno-import  -Wnonnull  -Wnull_termination  -Wpacked  -Wpadded @gol
                                 ^ typo (should be - not _)

-- 
Joseph S. Myers
jsm28@cam.ac.uk


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