Putting "unsigned" in anything but the first position is weird. As such,
tracetool's Rust type conversion will not support it. Remove it from
the whole of QEMU's source code, not just trace-events.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
crypto/pbkdf-gcrypt.c | 2 +-
crypto/pbkdf-gnutls.c | 2 +-
crypto/pbkdf-nettle.c | 2 +-
hw/display/exynos4210_fimd.c | 2 +-
hw/misc/imx7_src.c | 4 ++--
hw/net/can/can_sja1000.c | 4 ++--
hw/xen/trace-events | 4 ++--
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
index e89b8b1c768..f93996f674c 100644
--- a/crypto/pbkdf-gcrypt.c
+++ b/crypto/pbkdf-gcrypt.c
@@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
if (iterations > ULONG_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu must be less than %lu",
- (long long unsigned)iterations, ULONG_MAX);
+ (unsigned long long)iterations, ULONG_MAX);
return -1;
}
diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
index f34423f918b..46a3a869994 100644
--- a/crypto/pbkdf-gnutls.c
+++ b/crypto/pbkdf-gnutls.c
@@ -62,7 +62,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
if (iterations > ULONG_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu must be less than %lu",
- (long long unsigned)iterations, ULONG_MAX);
+ (unsigned long long)iterations, ULONG_MAX);
return -1;
}
diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
index 3ef9c1b52c4..3c8bbaf9f17 100644
--- a/crypto/pbkdf-nettle.c
+++ b/crypto/pbkdf-nettle.c
@@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
if (iterations > UINT_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu must be less than %u",
- (long long unsigned)iterations, UINT_MAX);
+ (unsigned long long)iterations, ULONG_MAX);
return -1;
}
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index c61e0280a7c..5632aa1388c 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1380,7 +1380,7 @@ static void exynos4210_fimd_write(void *opaque, hwaddr offset,
uint32_t old_value;
DPRINT_L2("write offset 0x%08x, value=%llu(0x%08llx)\n", offset,
- (long long unsigned int)val, (long long unsigned int)val);
+ (unsigned long long)val, (unsigned long long)val);
switch (offset) {
case FIMD_VIDCON0:
diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
index df0b0a69057..817c95bf65b 100644
--- a/hw/misc/imx7_src.c
+++ b/hw/misc/imx7_src.c
@@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
{
IMX7SRCState *s = (IMX7SRCState *)opaque;
uint32_t index = offset >> 2;
- long unsigned int change_mask;
+ uint32_t change_mask;
uint32_t current_value = value;
if (index >= SRC_MAX) {
@@ -180,7 +180,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
trace_imx7_src_write(imx7_src_reg_name(SRC_A7RCR0), s->regs[SRC_A7RCR0]);
- change_mask = s->regs[index] ^ (uint32_t)current_value;
+ change_mask = s->regs[index] ^ current_value;
switch (index) {
case SRC_A7RCR0:
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 5b6ba9df6c4..545c520c3b4 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
break;
}
}
- DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
- (int)addr, size, (long unsigned int)temp);
+ DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
+ (int)addr, size, (unsigned)temp);
return temp;
}
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index b67942d07b4..3b71ee641ff 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -57,8 +57,8 @@ cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uin
cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
cpu_get_ioreq_from_shared_memory_req_not_ready(int state, int data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O request not ready: 0x%x, ptr: 0x%x, port: 0x%"PRIx64", data: 0x%"PRIx64", count: %u, size: %u"
xen_main_loop_prepare_init_cpu(int id, void *cpu) "cpu_by_vcpu_id[%d]=%p"
-xen_map_ioreq_server_shared_page(long unsigned int ioreq_pfn) "shared page at pfn 0x%lx"
-xen_map_ioreq_server_buffered_io_page(long unsigned int ioreq_pfn) "buffered io page at pfn 0x%lx"
+xen_map_ioreq_server_shared_page(unsigned long int ioreq_pfn) "shared page at pfn 0x%lx"
+xen_map_ioreq_server_buffered_io_page(unsigned long int ioreq_pfn) "buffered io page at pfn 0x%lx"
xen_map_ioreq_server_buffered_io_evtchn(int bufioreq_evtchn) "buffered io evtchn is 0x%x"
destroy_hvm_domain_cannot_acquire_handle(void) "Cannot acquire xenctrl handle"
destroy_hvm_domain_failed_action(const char *action, int sts, char *errno_s) "xc_domain_shutdown failed to issue %s, sts %d, %s"
--
2.50.1On Fri, Aug 22, 2025 at 02:26:42PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:42 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 01/14] treewide: write "unsigned long int" instead of "long
> unsigned int"
> X-Mailer: git-send-email 2.50.1
>
> Putting "unsigned" in anything but the first position is weird.
I think one reason may be gcc uses something like ‘long unsigned int *‘
by default?
../hw/misc/imx7_src.c: In function ‘imx7_src_write’:
../hw/misc/imx7_src.c:218:42: error: passing argument 2 of ‘clear_bit’ from incompatible pointer type [-Werror=incompatible-pointer-types]
218 | clear_bit(R_CORE1_RST_SHIFT, &change_mask);
| ^~~~~~~~~~~~
| |
| uint32_t * {aka unsigned int *}
In file included from /media/liuzhao/data/qemu-cook/include/qemu/bitmap.h:16,
from /media/liuzhao/data/qemu-cook/include/hw/qdev-core.h:6,
from /media/liuzhao/data/qemu-cook/include/hw/sysbus.h:6,
from /media/liuzhao/data/qemu-cook/include/hw/misc/imx7_src.h:13,
from ../hw/misc/imx7_src.c:12:
/qemu/include/qemu/bitops.h:93:54: note: expected ‘long unsigned int *’ but argument is of type ‘uint32_t *’ {aka ‘unsigned int *’}
93 | static inline void clear_bit(long nr, unsigned long *addr)
| ~~~~~~~~~~~~~~~^~~~
cc1: all warnings being treated as errors
> As such,
> tracetool's Rust type conversion will not support it. Remove it from
> the whole of QEMU's source code, not just trace-events.
But I also agree it's a good idea to clean this up.
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> crypto/pbkdf-gcrypt.c | 2 +-
> crypto/pbkdf-gnutls.c | 2 +-
> crypto/pbkdf-nettle.c | 2 +-
> hw/display/exynos4210_fimd.c | 2 +-
> hw/misc/imx7_src.c | 4 ++--
> hw/net/can/can_sja1000.c | 4 ++--
> hw/xen/trace-events | 4 ++--
> 7 files changed, 10 insertions(+), 10 deletions(-)
...
> diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> index df0b0a69057..817c95bf65b 100644
> --- a/hw/misc/imx7_src.c
> +++ b/hw/misc/imx7_src.c
> @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
> {
> IMX7SRCState *s = (IMX7SRCState *)opaque;
> uint32_t index = offset >> 2;
> - long unsigned int change_mask;
> + uint32_t change_mask;
We needs "unsigned long", otherwise, there'll be the error as I listed
above.
> uint32_t current_value = value;
>
> if (index >= SRC_MAX) {
...
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 5b6ba9df6c4..545c520c3b4 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
> break;
> }
> }
> - DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> - (int)addr, size, (long unsigned int)temp);
tmep is "uint64_t", so there's no need to convert its type?
We can just drop `(long unsigned int)` directly.
> + DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
> + (int)addr, size, (unsigned)temp);
> return temp;
> }
Others look fine to me. With the nits fixed,
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
On Mon, Aug 25, 2025 at 03:24:01PM +0800, Zhao Liu wrote:
> On Fri, Aug 22, 2025 at 02:26:42PM +0200, Paolo Bonzini wrote:
> > Date: Fri, 22 Aug 2025 14:26:42 +0200
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH 01/14] treewide: write "unsigned long int" instead of "long
> > unsigned int"
> > X-Mailer: git-send-email 2.50.1
> >
> > Putting "unsigned" in anything but the first position is weird.
>
> I think one reason may be gcc uses something like ‘long unsigned int *‘
> by default?
>
> ../hw/misc/imx7_src.c: In function ‘imx7_src_write’:
> ../hw/misc/imx7_src.c:218:42: error: passing argument 2 of ‘clear_bit’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> 218 | clear_bit(R_CORE1_RST_SHIFT, &change_mask);
> | ^~~~~~~~~~~~
> | |
> | uint32_t * {aka unsigned int *}
> In file included from /media/liuzhao/data/qemu-cook/include/qemu/bitmap.h:16,
> from /media/liuzhao/data/qemu-cook/include/hw/qdev-core.h:6,
> from /media/liuzhao/data/qemu-cook/include/hw/sysbus.h:6,
> from /media/liuzhao/data/qemu-cook/include/hw/misc/imx7_src.h:13,
> from ../hw/misc/imx7_src.c:12:
> /qemu/include/qemu/bitops.h:93:54: note: expected ‘long unsigned int *’ but argument is of type ‘uint32_t *’ {aka ‘unsigned int *’}
> 93 | static inline void clear_bit(long nr, unsigned long *addr)
> | ~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
>
> > As such,
> > tracetool's Rust type conversion will not support it. Remove it from
> > the whole of QEMU's source code, not just trace-events.
>
> But I also agree it's a good idea to clean this up.
>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > crypto/pbkdf-gcrypt.c | 2 +-
> > crypto/pbkdf-gnutls.c | 2 +-
> > crypto/pbkdf-nettle.c | 2 +-
> > hw/display/exynos4210_fimd.c | 2 +-
> > hw/misc/imx7_src.c | 4 ++--
> > hw/net/can/can_sja1000.c | 4 ++--
> > hw/xen/trace-events | 4 ++--
> > 7 files changed, 10 insertions(+), 10 deletions(-)
>
> ...
>
> > diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> > index df0b0a69057..817c95bf65b 100644
> > --- a/hw/misc/imx7_src.c
> > +++ b/hw/misc/imx7_src.c
> > @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
> > {
> > IMX7SRCState *s = (IMX7SRCState *)opaque;
> > uint32_t index = offset >> 2;
> > - long unsigned int change_mask;
> > + uint32_t change_mask;
>
> We needs "unsigned long", otherwise, there'll be the error as I listed
> above.
>
> > uint32_t current_value = value;
> >
> > if (index >= SRC_MAX) {
>
> ...
>
> > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> > index 5b6ba9df6c4..545c520c3b4 100644
> > --- a/hw/net/can/can_sja1000.c
> > +++ b/hw/net/can/can_sja1000.c
> > @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
> > break;
> > }
> > }
> > - DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> > - (int)addr, size, (long unsigned int)temp);
>
> tmep is "uint64_t", so there's no need to convert its type?
We can't assume 'uint64_t' is a match for '%lx' - the
format string can be changed to PRIx64 though which
would let us drop the cast.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, 26 Aug 2025 at 12:16, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Aug 25, 2025 at 03:24:01PM +0800, Zhao Liu wrote:
> > > - DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> > > - (int)addr, size, (long unsigned int)temp);
> >
> > tmep is "uint64_t", so there's no need to convert its type?
>
> We can't assume 'uint64_t' is a match for '%lx' - the
> format string can be changed to PRIx64 though which
> would let us drop the cast.
Yes, we have quite a few uses of casts in printfs which
are really only there because the standard way to print
a uint64_t is so awkward and ugly...
-- PMM
On Fri, Aug 22, 2025 at 3:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Putting "unsigned" in anything but the first position is weird. As such,
> tracetool's Rust type conversion will not support it. Remove it from
> the whole of QEMU's source code, not just trace-events.
>
Hm weird C quirk indeed.
Why can't tracetool support this? Can't we just add the permutations
in the C_TO_RUST_TYPE_MAP dict in "[PATCH 06/14] tracetool: Add Rust
format support"?
+ "unsigned long long": "std::ffi::c_ulonglong",
+ "long unsigned long": "std::ffi::c_ulonglong",
+ "long long unsigned": "std::ffi::c_ulonglong",
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> crypto/pbkdf-gcrypt.c | 2 +-
> crypto/pbkdf-gnutls.c | 2 +-
> crypto/pbkdf-nettle.c | 2 +-
> hw/display/exynos4210_fimd.c | 2 +-
> hw/misc/imx7_src.c | 4 ++--
> hw/net/can/can_sja1000.c | 4 ++--
> hw/xen/trace-events | 4 ++--
> 7 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
> index e89b8b1c768..f93996f674c 100644
> --- a/crypto/pbkdf-gcrypt.c
> +++ b/crypto/pbkdf-gcrypt.c
> @@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
> if (iterations > ULONG_MAX) {
> error_setg_errno(errp, ERANGE,
> "PBKDF iterations %llu must be less than %lu",
> - (long long unsigned)iterations, ULONG_MAX);
> + (unsigned long long)iterations, ULONG_MAX);
> return -1;
> }
>
> diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
> index f34423f918b..46a3a869994 100644
> --- a/crypto/pbkdf-gnutls.c
> +++ b/crypto/pbkdf-gnutls.c
> @@ -62,7 +62,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
> if (iterations > ULONG_MAX) {
> error_setg_errno(errp, ERANGE,
> "PBKDF iterations %llu must be less than %lu",
> - (long long unsigned)iterations, ULONG_MAX);
> + (unsigned long long)iterations, ULONG_MAX);
> return -1;
> }
>
> diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
> index 3ef9c1b52c4..3c8bbaf9f17 100644
> --- a/crypto/pbkdf-nettle.c
> +++ b/crypto/pbkdf-nettle.c
> @@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
> if (iterations > UINT_MAX) {
> error_setg_errno(errp, ERANGE,
> "PBKDF iterations %llu must be less than %u",
> - (long long unsigned)iterations, UINT_MAX);
> + (unsigned long long)iterations, ULONG_MAX);
> return -1;
> }
>
> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> index c61e0280a7c..5632aa1388c 100644
> --- a/hw/display/exynos4210_fimd.c
> +++ b/hw/display/exynos4210_fimd.c
> @@ -1380,7 +1380,7 @@ static void exynos4210_fimd_write(void *opaque, hwaddr offset,
> uint32_t old_value;
>
> DPRINT_L2("write offset 0x%08x, value=%llu(0x%08llx)\n", offset,
> - (long long unsigned int)val, (long long unsigned int)val);
> + (unsigned long long)val, (unsigned long long)val);
>
> switch (offset) {
> case FIMD_VIDCON0:
> diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> index df0b0a69057..817c95bf65b 100644
> --- a/hw/misc/imx7_src.c
> +++ b/hw/misc/imx7_src.c
> @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
> {
> IMX7SRCState *s = (IMX7SRCState *)opaque;
> uint32_t index = offset >> 2;
> - long unsigned int change_mask;
> + uint32_t change_mask;
> uint32_t current_value = value;
>
> if (index >= SRC_MAX) {
> @@ -180,7 +180,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
>
> trace_imx7_src_write(imx7_src_reg_name(SRC_A7RCR0), s->regs[SRC_A7RCR0]);
>
> - change_mask = s->regs[index] ^ (uint32_t)current_value;
> + change_mask = s->regs[index] ^ current_value;
>
> switch (index) {
> case SRC_A7RCR0:
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 5b6ba9df6c4..545c520c3b4 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
> break;
> }
> }
> - DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> - (int)addr, size, (long unsigned int)temp);
> + DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
> + (int)addr, size, (unsigned)temp);
>
> return temp;
> }
> diff --git a/hw/xen/trace-events b/hw/xen/trace-events
> index b67942d07b4..3b71ee641ff 100644
> --- a/hw/xen/trace-events
> +++ b/hw/xen/trace-events
> @@ -57,8 +57,8 @@ cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uin
> cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
> cpu_get_ioreq_from_shared_memory_req_not_ready(int state, int data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O request not ready: 0x%x, ptr: 0x%x, port: 0x%"PRIx64", data: 0x%"PRIx64", count: %u, size: %u"
> xen_main_loop_prepare_init_cpu(int id, void *cpu) "cpu_by_vcpu_id[%d]=%p"
> -xen_map_ioreq_server_shared_page(long unsigned int ioreq_pfn) "shared page at pfn 0x%lx"
> -xen_map_ioreq_server_buffered_io_page(long unsigned int ioreq_pfn) "buffered io page at pfn 0x%lx"
> +xen_map_ioreq_server_shared_page(unsigned long int ioreq_pfn) "shared page at pfn 0x%lx"
> +xen_map_ioreq_server_buffered_io_page(unsigned long int ioreq_pfn) "buffered io page at pfn 0x%lx"
> xen_map_ioreq_server_buffered_io_evtchn(int bufioreq_evtchn) "buffered io evtchn is 0x%x"
> destroy_hvm_domain_cannot_acquire_handle(void) "Cannot acquire xenctrl handle"
> destroy_hvm_domain_failed_action(const char *action, int sts, char *errno_s) "xc_domain_shutdown failed to issue %s, sts %d, %s"
> --
> 2.50.1
>
>
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
On Mon, Aug 25, 2025 at 8:40 AM Manos Pitsidianakis <manos.pitsidianakis@linaro.org> wrote: > Why can't tracetool support this? Can't we just add the permutations > in the C_TO_RUST_TYPE_MAP dict in "[PATCH 06/14] tracetool: Add Rust > format support"? > > + "unsigned long long": "std::ffi::c_ulonglong", > + "long unsigned long": "std::ffi::c_ulonglong", > + "long long unsigned": "std::ffi::c_ulonglong", Just laziness. I guess I can just sort the keywords. Paolo
© 2016 - 2025 Red Hat, Inc.