qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] bswap.h: Fix const_le64() macro


From: Thomas Huth
Subject: Re: [PATCH] bswap.h: Fix const_le64() macro
Date: Mon, 22 Jan 2024 18:47:20 +0100
User-agent: Mozilla Thunderbird

On 22/01/2024 18.37, Peter Maydell wrote:
The const_le64() macro introduced in commit 845d80a8c7b187 turns out
to have a bug which means that on big-endian systems the compiler
complains if the argument isn't already a 64-bit type. This hasn't
caused a problem yet, because there are no in-tree uses, but it
means it's not possible for anybody to add one without it failing CI.

This example is from an attempted use of it with the argument '0',
from the s390 CI runner's gcc:

../block/blklogwrites.c: In function ‘blk_log_writes_co_do_log’:
../include/qemu/bswap.h:148:36: error: left shift count >= width of
type [-Werror=shift-count-overflow]
148 | ((((_x) & 0x00000000000000ffU) << 56) | \
| ^~
../block/blklogwrites.c:409:27: note: in expansion of macro ‘const_le64’
409 | .nr_entries = const_le64(0),
| ^~~~~~~~~~
../include/qemu/bswap.h:149:36: error: left shift count >= width of
type [-Werror=shift-count-overflow]
149 | (((_x) & 0x000000000000ff00U) << 40) | \
| ^~
../block/blklogwrites.c:409:27: note: in expansion of macro ‘const_le64’
409 | .nr_entries = const_le64(0),
| ^~~~~~~~~~
cc1: all warnings being treated as errors

Fix this by making all the constants in the macro have the ULL
suffix.  This will cause them all to be 64-bit integers, which means
the result of the logical & will also be an unsigned 64-bit type,
even if the input to the macro is a smaller type, and so the shifts
will be in range.

Fixes: 845d80a8c7b187 ("qemu/bswap: Add const_le64()")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Tested 'by hand' on the s390 box that was complaining about
Kevin's pullreq.
---
  include/qemu/bswap.h | 16 ++++++++--------
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 933a66ee87e..bd67468e5e4 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -145,14 +145,14 @@ CPU_CONVERT(le, 64, uint64_t)
   */
  #if HOST_BIG_ENDIAN
  # define const_le64(_x)                          \
-    ((((_x) & 0x00000000000000ffU) << 56) |      \
-     (((_x) & 0x000000000000ff00U) << 40) |      \
-     (((_x) & 0x0000000000ff0000U) << 24) |      \
-     (((_x) & 0x00000000ff000000U) <<  8) |      \
-     (((_x) & 0x000000ff00000000U) >>  8) |      \
-     (((_x) & 0x0000ff0000000000U) >> 24) |      \
-     (((_x) & 0x00ff000000000000U) >> 40) |      \
-     (((_x) & 0xff00000000000000U) >> 56))
+    ((((_x) & 0x00000000000000ffULL) << 56) |    \
+     (((_x) & 0x000000000000ff00ULL) << 40) |    \
+     (((_x) & 0x0000000000ff0000ULL) << 24) |    \
+     (((_x) & 0x00000000ff000000ULL) <<  8) |    \
+     (((_x) & 0x000000ff00000000ULL) >>  8) |    \
+     (((_x) & 0x0000ff0000000000ULL) >> 24) |    \
+     (((_x) & 0x00ff000000000000ULL) >> 40) |    \
+     (((_x) & 0xff00000000000000ULL) >> 56))
  # define const_le32(_x)                          \
      ((((_x) & 0x000000ffU) << 24) |              \
       (((_x) & 0x0000ff00U) <<  8) |              \

Reviewed-by: Thomas Huth <thuth@redhat.com>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]