[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] non-null declarations
From: |
Eric Blake |
Subject: |
Re: [PATCH] non-null declarations |
Date: |
Thu, 10 Dec 2009 16:06:32 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Bruno Haible <bruno <at> clisp.org> writes:
> Two points I'm not sure about:
> - Should the macro be called _GL_ARG_NONNULL or GL_ARG_NONNULL?
_GL_ARG_NONNULL is in the implementation-reserved namespace, so it is less
likely to collide, but it is also more to type. I don't really care one way or
another.
> - Is it worth putting the macro definition (always the same 10 lines of
code)
> into a separate file, like done with link-warning.h?
I'd go one step further - I'd like to see it in config.h, as was done with
_UNUSED_PARAMETER_ in the AH_VERBATIM block of gnulib-common.m4. In other
words, it is something so frequently used that it is nicer to guarantee that it
is always defined (if you followed the instructions of including <config.h>
first).
> extern int scandir (const char *dir, struct dirent ***namelist,
> int (*filter) (const struct dirent *),
> - int (*cmp) (const struct dirent **, const struct dirent
**));
> + int (*cmp) (const struct dirent **, const struct dirent **))
> + _GL_ARG_NONNULL ((1, 2, 4));
Is there any desire to provide guaranteed extension semantics that cmp can be
NULL in order to get an unsorted subset returned?
> -extern locale_t duplocale (locale_t locale);
> +extern locale_t duplocale (locale_t locale) _GL_ARG_NONNULL ((1));
Are we guaranteed that locale_t is a pointer type; or can it be a scalar on
some platforms, in which case _GL_ARG_NONNULL might cause grief?
> extern int fprintf (FILE *fp, const char *format, ...)
> - __attribute__ ((__format__ (__printf__, 2, 3)));
> + __attribute__ ((__format__ (__printf__, 2, 3))) _GL_ARG_NONNULL ((2));
Missed arg 1 (in two declarations of fprintf).
> -extern FILE * freopen (const char *filename, const char *mode, FILE *stream);
> +extern FILE * freopen (const char *filename, const char *mode, FILE *stream)
> + _GL_ARG_NONNULL ((1, 2, 3));
Bug: Arg 1 should not be listed. Cygwin DEPENDS on being able to pass NULL
filename to change streams from text to binary.
> extern int snprintf (char *str, size_t size, const char *format, ...)
> - __attribute__ ((__format__ (__printf__, 3, 4)));
> + __attribute__ ((__format__ (__printf__, 3, 4)))
> + _GL_ARG_NONNULL ((1, 3));
Bug: Arg 1 should not be listed. POSIX requires that str may be null if size
is 0.
> -extern int setenv (const char *name, const char *value, int replace);
> +extern int setenv (const char *name, const char *value, int replace)
> + _GL_ARG_NONNULL ((1));
POSIX and gnulib guarantee that calling this with NAME set to NULL will
gracefully fail with EINVAL, so maybe we shouldn't list this one. On the other
hand, the POSIX folks are considering relaxing this (at my suggestion), so if
we DO keep this nonnull decoration, then I need to do a corresponding
relaxation of the testsuite to not pass NULL in the first place.
http://austingroupbugs.net/view.php?id=185
> -extern int unsetenv (const char *name);
> +extern int unsetenv (const char *name) _GL_ARG_NONNULL ((1));
Same as for setenv.
> extern void *memchr (void const *__s, int __c, size_t __n)
> - __attribute__ ((__pure__));
> + __attribute__ ((__pure__)) _GL_ARG_NONNULL ((1));
This one I'm fuzzy on. On one hand, the POSIX definition is that the function
reads at most __c bytes, so when __c is 0, it is safe for __s to be NULL. On
the other hand, glibc already warns here, the gnulib testsuite already jumps
through hoops to avoid the warning, and the likelihood of meaning to pass NULL
when __c is 0 is slim. Your call.
> extern void *memmem (void const *__haystack, size_t __haystack_len,
> void const *__needle, size_t __needle_len)
> - __attribute__ ((__pure__));
> + __attribute__ ((__pure__)) _GL_ARG_NONNULL ((1, 3));
Same goes for all of the mem* interfaces, when length is 0.
> -extern int rpl_recvfrom (int, void *, int, int, struct sockaddr *, int *);
> +extern int rpl_recvfrom (int, void *, int, int, struct sockaddr *, int *)
> + _GL_ARG_NONNULL ((2, 6));
Bug: Arg 6 can be NULL, since POSIX states that "The recv( ) function is
equivalent to recvfrom( ) with a zero address_len argument" (well, technically,
a null address_len argument only makes sense when address [arg 5] is also NULL).
> -extern int rpl_sendto (int, const void *, int, int, struct sockaddr *, int);
> +extern int rpl_sendto (int, const void *, int, int, struct sockaddr *, int)
> + _GL_ARG_NONNULL ((2, 5));
Bug: Arg 5 can be NULL, since POSIX states that "The send( ) function is
equivalent to sendto( ) with a null pointer dest_len argument" (well, that's
also a bug in POSIX, since dest_len is socklen_t; it obviously meant a null
dest_addr argument).
> -extern int futimens (int fd, struct timespec const times[2]);
> +extern int futimens (int fd, struct timespec const times[2])
> + _GL_ARG_NONNULL ((2));
Bug: Arg 2 can be NULL, to specify 'now'. This function should not have a
nonnull decoration.
> extern int utimensat (int fd, char const *name,
> - struct timespec const times[2], int flag);
> + struct timespec const times[2], int flag)
> + _GL_ARG_NONNULL ((2, 3));
Likewise, Arg 3 should not be listed.
--
Eric Blake
Re: [PATCH] non-null declarations,
Eric Blake <=
Re: [PATCH] non-null declarations, Bruno Haible, 2009/12/10
Re: non-null declarations, recvfrom, sendto, Bruno Haible, 2009/12/10
Re: non-null declarations, recvfrom, sendto, Paolo Bonzini, 2009/12/10
Re: [PATCH] non-null declarations, Dmitry V. Levin, 2009/12/10