Re: [PATCH 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings

[Daniel Henrique Barboza]

Joel,

I'll make these changes in this patch to be consistent with what we've
been discussing:
- change addr to hwaddr
- use HWADDR_PRIx instead of PRIx64

i.e. this diff:

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 1eae84db15..0020d8f404 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -303,12 +303,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int 
socket,
 static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
 {
     g_autofree char *mem_name = NULL;
-    uint64_t addr, size;
+    hwaddr addr;
+    uint64_t size;
     MachineState *ms = MACHINE(s);
 
     addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
     size = riscv_socket_mem_size(ms, socket);
-    mem_name = g_strdup_printf("/memory@%"PRIx64, addr);
+    mem_name = g_strdup_printf("/memory@%"HWADDR_PRIx, addr);
     qemu_fdt_add_subnode(ms->fdt, mem_name);
     qemu_fdt_setprop_cells(ms->fdt, mem_name, "reg",
         addr >> 32, addr, size >> 32, size);


I did this change and applied your "hw/riscv/virt: device tree reg cleanups" 
series on top
of it, and there are no conflicts. No change needed in your side.


Thanks,

Daniel



On 4/24/25 6:41 AM, Joel Stanley wrote:
On Wed, 23 Apr 2025 at 20:39, Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:

We can avoid the 'long' casts by using PRIx64 and HWADDR_PRIx on the fmt
strings for uint64_t and hwaddr types.

Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
  hw/riscv/virt.c | 29 +++++++++++++++--------------
  1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 036a0a9bfb..075c035f25 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -309,7 +309,7 @@ static void create_fdt_socket_memory(RISCVVirtState *s, int 
socket)

      addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
      size = riscv_socket_mem_size(ms, socket);
-    mem_name = g_strdup_printf("/memory@%lx", (long)addr);
+    mem_name = g_strdup_printf("/memory@%"PRIx64, addr);

I wondered why this wasn't a HWADDR_PRIx.

addr (and NodeInfo::node_mem?) could be a hwaddr? That would make
everything more consistent.

Cheers,

Joel