Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Zhao Liu <zhao1.liu@intel.com>]

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 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>