Paul Eggert wrote: > Problem found by xlclang 16.1 on AIX 7.2. > * lib/localename.c (duplocale, freelocale): > Omit unnecessary comparison of non-null args to NULL.
I disagree with this patch. Compiler warnings are supposed to help us improve the code. Replacing a function that starts with an entry check — which is a good practice [1] — with one that operates in garbage-in - garbage-out fashion is not an improvement; quite the opposite. We already know how to handle this situation: see canonicalize-lgpl.c execl.c execle.c execlp.c execve.c execvpe.c getaddrinfo.c getdelim.c getpass.c glob.c random_r.c setenv.c tsearch.c unsetenv.c [1] https://cwe.mitre.org/data/definitions/20.html 2023-01-13 Bruno Haible <br...@clisp.org> localename: Fix -Wtautological-pointer-compare warning in a better way. * lib/localename.c (duplocale, freelocale): Revert last patch. (_GL_ARG_NONNULL): Define to empty. diff --git a/lib/localename.c b/lib/localename.c index 5a178c68fe..8fe90e0bf2 100644 --- a/lib/localename.c +++ b/lib/localename.c @@ -18,6 +18,12 @@ /* Native Windows code written by Tor Lillqvist <t...@iki.fi>. */ /* Mac OS X code written by Bruno Haible <br...@clisp.org>. */ +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + optimizes away the locale == NULL tests below in duplocale() and freelocale(), + or xlclang reports -Wtautological-pointer-compare warnings for these tests. + */ +#define _GL_ARG_NONNULL(params) + #include <config.h> /* Specification. */ @@ -2967,6 +2973,10 @@ duplocale (locale_t locale) struct locale_hash_node *node; locale_t result; + if (locale == NULL) + /* Invalid argument. */ + abort (); + node = (struct locale_hash_node *) malloc (sizeof (struct locale_hash_node)); if (node == NULL) /* errno is set to ENOMEM. */ @@ -3052,7 +3062,7 @@ void freelocale (locale_t locale) #undef freelocale { - if (locale == LC_GLOBAL_LOCALE) + if (locale == NULL || locale == LC_GLOBAL_LOCALE) /* Invalid argument. */ abort ();
