On 2024-05-17 16:51, Bruno Haible wrote:
-------------------------- foo.c --------------------------
unsigned long long x = 0xff00000000000000;
-----------------------------------------------------------
$ gcc -Wall -S foo.c
foo.c:1: warning: integer constant is too large for ‘long’ type
I don't see how that would produce incorrect code for byteswap.h.
If memory serves, GCC formerly warned about integer constants that had a
"surprising" type, for a definition of "surprising" that was too
generous so that there were too many false alarms.
The longer story is C89 lacked 'long long'. Also, in C89 on a 32-bit int
platform the decimal integer literal 2147483648 had type 'unsigned int'
because 2147483648 fits in 'unsigned int' but not plain 'int'. Although
this misfeature was fixed in C99 it was present in many compilers for
some time after that. (It still seems to be present in MS Visual Studio!
See
<https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4146?view=msvc-170>.)
To help write portable code, GCC formerly warned about integer constants
that might not be portable to C89, but it then went ahead and produced
correct code.
The need for these warnings went away long ago (except perhaps for
Microsoft...) but they can be annoying so I installed the attached patch
to pacify older GCCs.
PS. We could simplify the code by removing all uses of __builtin_bswap64
etc. Although these uses are present only to improve performance, they
do not improve performance with GCC 14 x86-64 with -O2.From 221d026428e20d18faf96b0e38b25f4d89d32805 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 18 May 2024 07:48:47 -0700
Subject: [PATCH] byteswap: pacify GCC 4.4.7 and older
Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2024-05/msg00277.html
* lib/byteswap.in.h (bswap_16, bswap_32, bswap_64):
Compute the mask rather than using long constants like
0xff00000000000000 that may generate bogus warnings.
---
ChangeLog | 9 +++++++++
lib/byteswap.in.h | 31 +++++++++++++++++--------------
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 29adf56ab7..79f813e1b2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-05-18 Paul Eggert <egg...@cs.ucla.edu>
+
+ byteswap: pacify GCC 4.4.7 and older
+ Problem reported by Bruno Haible in:
+ https://lists.gnu.org/r/bug-gnulib/2024-05/msg00277.html
+ * lib/byteswap.in.h (bswap_16, bswap_32, bswap_64):
+ Compute the mask rather than using long constants like
+ 0xff00000000000000 that may generate bogus warnings.
+
2024-05-18 Bruno Haible <br...@clisp.org>
endian tests: Verify that it can be used from C++.
diff --git a/lib/byteswap.in.h b/lib/byteswap.in.h
index 0e760005c2..4be335d915 100644
--- a/lib/byteswap.in.h
+++ b/lib/byteswap.in.h
@@ -62,8 +62,9 @@ bswap_16 (uint_least16_t x)
#ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP16
return __builtin_bswap16 (x);
#else
- return ( (x & 0xff00) >> 8
- | (x & 0x00ff) << 8);
+ uint_fast16_t mask = 0xff;
+ return ( (x & mask << 8 * 1) >> 8 * 1
+ | (x & mask << 8 * 0) << 8 * 1);
#endif
}
@@ -75,10 +76,11 @@ bswap_32 (uint_least32_t x)
#ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP32
return __builtin_bswap32 (x);
#else
- return ( (x & 0xff000000) >> 24
- | (x & 0x00ff0000) >> 8
- | (x & 0x0000ff00) << 8
- | (x & 0x000000ff) << 24);
+ uint_fast32_t mask = 0xff;
+ return ( (x & mask << 8 * 3) >> 8 * 3
+ | (x & mask << 8 * 2) >> 8 * 1
+ | (x & mask << 8 * 1) << 8 * 1
+ | (x & mask << 8 * 0) << 8 * 3);
#endif
}
@@ -91,14 +93,15 @@ bswap_64 (uint_least64_t x)
# ifdef _GL_BYTESWAP_HAS_BUILTIN_BSWAP64
return __builtin_bswap64 (x);
# else
- return ( (x & 0xff00000000000000) >> 56
- | (x & 0x00ff000000000000) >> 40
- | (x & 0x0000ff0000000000) >> 24
- | (x & 0x000000ff00000000) >> 8
- | (x & 0x00000000ff000000) << 8
- | (x & 0x0000000000ff0000) << 24
- | (x & 0x000000000000ff00) << 40
- | (x & 0x00000000000000ff) << 56);
+ uint_fast64_t mask = 0xff;
+ return ( (x & mask << 8 * 7) >> 8 * 7
+ | (x & mask << 8 * 6) >> 8 * 5
+ | (x & mask << 8 * 5) >> 8 * 3
+ | (x & mask << 8 * 4) >> 8 * 1
+ | (x & mask << 8 * 3) << 8 * 1
+ | (x & mask << 8 * 2) << 8 * 3
+ | (x & mask << 8 * 1) << 8 * 5
+ | (x & mask << 8 * 0) << 8 * 7);
# endif
}
#endif
--
2.40.1