On Tue, 30 Dec 2025, Philippe Mathieu-Daudé wrote:
On 28/12/25 21:04, Philippe Mathieu-Daudé wrote:
Hi Zoltan,
On 28/12/25 17:46, BALATON Zoltan wrote:
On Sun, 28 Dec 2025, Philippe Mathieu-Daudé wrote:
Unaligned memcpy API is buried within 'qemu/bswap.h',
supposed to be related to endianness swapping. Extract
to a new header to clarify.
Suggested-by: Paolo Bonzini <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
include/qemu/bswap.h | 62 +------------------------
include/qemu/memory_ldst_unaligned.h | 67 ++++++++++++++++++++++++++++
accel/tcg/translator.c | 1 +
hw/display/ati_2d.c | 1 +
hw/display/sm501.c | 2 +-
hw/remote/vfio-user-obj.c | 1 +
hw/vmapple/virtio-blk.c | 1 +
net/checksum.c | 1 +
ui/vnc-enc-tight.c | 1 +
util/bufferiszero.c | 2 +-
10 files changed, 76 insertions(+), 63 deletions(-)
create mode 100644 include/qemu/memory_ldst_unaligned.h
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index b77ea955de5..e8b944988c3 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -1,6 +1,7 @@
#ifndef BSWAP_H
#define BSWAP_H
+#include "qemu/memory_ldst_unaligned.h"
If it's included here do users need to also include it separately or if so
should some of those users lose bswap.h include now instead of including
both this header and bswap.h? I think it's simpler to only include it here
and let users get it through bswap.h unless you review and remove now
unnecessary bswap.h includes from places that only need this hearder but I
don't know if that's worth the effort.
bswap API users have to include <qemu/bswap.h>,
users of ld/st_unaligned() one have to include <qemu/
memory_ldst_unaligned.h>.
Let me try again.
Users of the ld/st_unaligned() API have to include
<qemu/ldst_unaligned.h>; if they don't use <qemu/bswap.h>
then first it is pointless to include it, but furthermore
it saves few compilation cycles for the unused inlined
bswap functions.
Users of the bswap() API have to include <qemu/bswap.h>
(which indirectly includes <qemu/ldst_unaligned.h>, but
this is irrelevant).
Users of both the bswap() and ld/st_unaligned() APIs have
to include both <qemu/bswap.h> AND <qemu/ldst_unaligned.h>,
even if it is indirectly included. That makes the use of
APIs more explicit in the source file, and furthermore it
avoids code churn when indirect headers are reworked.
Your patch added #include of the new header to 8 files so this is already
causing churn in the hope of avoiding it.
For recent examples see commits 34bcd8f4ff9 ("hw/int/loongarch:
Include missing 'system/memory.h' header") or fdda97b47e6
("hw/arm/virt-acpi-build: Include missing 'cpu.h' header").
Is it clearer?
I did not notice that you removed bswap.h from 2 files and thought the
patch only added the new header. In that case my comment does not apply
but I still see this more as a churn than useful cleanup. OK, host endian
does not swap but if you consider that the le and be functions in bswap.h
are based on the he functions then you can just leave these there. If it
bothers you add a comment, that's still less churn.
Regards,
BALATON Zoltan
