Paul Eggert wrote:
> As with any GCC diagnostic, there's a tradeoff between the hassle of
> fixing false positives and the benefit of finding flaky code. I hope
> that -Wuseless-cast is worth the hassle, at least for Gnulib.
In code where types are fixed (not platform-dependent or instantiation-
dependent), avoiding the casts may be OK.
But in platform-dependent code, omitting the casts often makes things
_more_ fragile and introduces _more_ diagnostics. In these cases,
the compound-literal approach is better than omitting the cast.
(The wchar_t mistake in your patches highlights this: wchar_t
is platform-dependent, therefore omitting casts to wchar_t was
likely to introduce bugs.)
In summary, we have three approaches to pacify -Wuseless-cast
warnings:
(A) Omit the cast.
(B) Use compound-initializer syntax: (TYPE) {EXPRESSION}
(C) Use the cast: (TYPE) (EXPRESSION),
and
#if 14 <= __GNUC__
# pragma GCC diagnostic ignored "-Wuseless-cast"
#endif
near the top of the compilation unit.
I'm glad that you brought up (B), because it is not well-known.
Yet it behaves like 'static_cast<>' in C++: It allows conversions between
numeric types but disallows conversions pointer <--> number.
(A) is appropriate when you can verify that the source and destination
types are the same, on all platforms, and in all configurations.
(B) is appropriate when the types vary depending on platform or
configuration, but they are numeric types always.
(C) is appropriate when the types are sometimes numeric and sometimes
pointers, or when dealing with the warnings on a case-by-case basis
is not worth the effort.
Let me comment on the patches individually.
* fstrcmp: pacify -Wuseless-cast
OK, although it makes things more brittle: what if 'bufmax' was
changed to a different type?
* Pacify -Wuseless-cast via compound literals in lib
Looks OK, except for the blunder in gettext.h that you already fixed.
* Pacify -Wuseless-cast via compound literals in tests
The change from
(wchar_t) 0xBADFACE
to
0xBADFACE
is OK in assignments to 'wc', but is buggy in comparisons such as
- ASSERT (wc == (wchar_t) 0xBADFACE);
+ ASSERT (wc == 0xBADFACE);
because on Windows and on 32-bit AIX, wc is 16 bit and therefore
expected to be == 0xFACEU, not == 0xBADFACE.
I fixed this now.
* wait-process: omit confusing cast
If some platform has a WTERMSIG macro that returns a 'long' or
'unsigned long', this causes a bug. We are talking about an stdargs
argument to a format string.
* jit/cache-tests: omit confusing cast.
Just because the test uses variables of type 'char *' does not mean
that SET_CODE's argument should always be a 'char *'. It could also
be a function pointer or an uintptr_t. Therefore the cast to 'char *'
serves a real purpose.
* sigsegv: pacify -Wuseless-cast
By design, SIGSEGV_FAULT_ADDRESS may return a pointer or an
uintptr_t or an intptr_t. Therefore the cast is *mandatory*, even if
on Linux/x86_64 it may look unnecessary.
* In tests, prefer %zu to %lu+cast
Agree: %zu is safe to use nowadays. Even MSVC has it.
* limits-h-tests: suppress -Wuseless-cast
I said that gcc 14 and 15 also have the option -Wuseless-cast.
2026-05-10 Bruno Haible <[email protected]>
Revisit some -Wuseless-cast changes.
* lib/fstrcmp.c (fstrcmp_free_resources, fstrcmp_bounded): Use a
compound literal to convert to uintptr_t.
* lib/wait-process.c (wait_subprocess): Use a compound literal to
convert to int.
* lib/sigsegv.c: Revert last change. Instead, disable -Wuseless-cast
in this compilation unit.
* tests/jit/test-cache.c: Likewise.
* tests/test-limits-h.c: Disable -Wuseless-cast warnings also for
gcc 14, 15.
diff --git a/lib/fstrcmp.c b/lib/fstrcmp.c
index 69aca5d0e7..5a1c273394 100644
--- a/lib/fstrcmp.c
+++ b/lib/fstrcmp.c
@@ -82,7 +82,7 @@ fstrcmp_free_resources (void)
if (buffer != NULL)
{
gl_tls_set (buffer_key, NULL);
- gl_tls_set (bufmax_key, (void *) (uintptr_t) 0);
+ gl_tls_set (bufmax_key, (void *) (uintptr_t) {0});
free (buffer);
}
}
@@ -214,7 +214,7 @@ fstrcmp_bounded (const char *string1, const char *string2,
double lower_bound)
free (buffer);
buffer = xnmalloc (bufmax, 2 * sizeof *buffer);
gl_tls_set (buffer_key, buffer);
- gl_tls_set (bufmax_key, (void *) bufmax);
+ gl_tls_set (bufmax_key, (void *) (uintptr_t) {bufmax});
}
ctxt.fdiag = buffer + yvec_length + 1;
ctxt.bdiag = ctxt.fdiag + fdiag_len;
diff --git a/lib/sigsegv.c b/lib/sigsegv.c
index cdedbf849d..ccd23bd5ba 100644
--- a/lib/sigsegv.c
+++ b/lib/sigsegv.c
@@ -35,6 +35,13 @@
# include <sys/param.h> /* defines macro OpenBSD */
#endif
+/* The value of SIGSEGV_FAULT_ADDRESS may be any representation of an address
+ (void *, char *, uintptr_t, intptr_t), depending on platform. The cast is
+ therefore mandatory, to avoid a compilation error or warning. */
+#if _GL_GNUC_PREREQ (14, 0)
+# pragma GCC diagnostic ignored "-Wuseless-cast"
+#endif
+
/* Version number. */
int libsigsegv_version = LIBSIGSEGV_VERSION;
@@ -362,7 +369,7 @@ int libsigsegv_version = LIBSIGSEGV_VERSION;
#if defined __gnu_hurd__ /* Hurd */
# define SIGSEGV_FAULT_HANDLER_ARGLIST int sig, long code, struct sigcontext
*scp
-# define SIGSEGV_FAULT_ADDRESS (void *) (unsigned long) code
+# define SIGSEGV_FAULT_ADDRESS (unsigned long) code
# define SIGSEGV_FAULT_CONTEXT scp
# if defined __x86_64__
@@ -1017,7 +1024,7 @@ static sigsegv_handler_t user_handler =
(sigsegv_handler_t)NULL;
static void
sigsegv_handler (SIGSEGV_FAULT_HANDLER_ARGLIST)
{
- void *address = SIGSEGV_FAULT_ADDRESS;
+ void *address = (void *) (SIGSEGV_FAULT_ADDRESS);
# if HAVE_STACK_OVERFLOW_RECOVERY
# if !(HAVE_STACKVMA || defined SIGSEGV_FAULT_STACKPOINTER)
@@ -1260,7 +1267,7 @@ install_for (int sig)
struct sigaction action;
# ifdef SIGSEGV_FAULT_ADDRESS_FROM_SIGINFO
- action.sa_sigaction = sigsegv_handler;
+ action.sa_sigaction = (void (*) (int, siginfo_t *, void *)) &sigsegv_handler;
# else
action.sa_handler = (void (*) (int)) &sigsegv_handler;
# endif
diff --git a/lib/wait-process.c b/lib/wait-process.c
index 2e3cf70a11..f8ce59a4c9 100644
--- a/lib/wait-process.c
+++ b/lib/wait-process.c
@@ -383,7 +383,7 @@ wait_subprocess (pid_t child, const char *progname,
if (exit_on_error || (!null_stderr && termsigp == NULL))
error (exit_on_error ? EXIT_FAILURE : 0, 0,
_("%s subprocess got fatal signal %d"),
- progname, WTERMSIG (status));
+ progname, (int) {WTERMSIG (status)});
return 127;
}
if (!WIFEXITED (status))
diff --git a/tests/jit/test-cache.c b/tests/jit/test-cache.c
index 8bf152f4ed..292287bd24 100644
--- a/tests/jit/test-cache.c
+++ b/tests/jit/test-cache.c
@@ -50,6 +50,12 @@
#endif
static int clang_ubsan_workaround = 0;
+/* This file freely casts between the different representations of addresses
+ (void *, char *, function pointers, uintptr_t). Not worth warning about.
*/
+#if _GL_GNUC_PREREQ (14, 0)
+# pragma GCC diagnostic ignored "-Wuseless-cast"
+#endif
+
/* On most platforms, function pointers are just a pointer to the
code, i.e. to the first instruction to be executed. This,
however, is not universally true, see:
@@ -116,7 +122,7 @@ struct func
# else
# define CODE(funcptr) ((char *) (funcptr) - clang_ubsan_workaround)
# define SET_CODE(funcptr,code_addr) \
- ((void) ((funcptr) = (void *) ((code_addr) + clang_ubsan_workaround)))
+ ((void) ((funcptr) = (void *) ((char *) (code_addr) +
clang_ubsan_workaround)))
# define IS(funcptr) ((void) (funcptr), 0)
# define SET_IS(funcptr,is) ((void) (funcptr), (void) (is))
# endif
diff --git a/tests/test-limits-h.c b/tests/test-limits-h.c
index 5688a3e2d3..8691caa32e 100644
--- a/tests/test-limits-h.c
+++ b/tests/test-limits-h.c
@@ -27,8 +27,8 @@
#define verify_width(width, min, max) \
static_assert ((max) >> ((width) - 1 - ((min) < 0)) == 1)
-/* Macros borrowed from intprops.h. */
-#if 16 <= __GNUC__
+/* Macros borrowed from intprops.h and intprops-internal.h. */
+#if 14 <= __GNUC__
# pragma GCC diagnostic ignored "-Wuseless-cast"
#endif
#define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
