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

Reply via email to