[PATCH 3/4] hw/arm: use g_auto(GStrv) for node_path in arm_load_dtb

Alex Bennée posted 4 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH 3/4] hw/arm: use g_auto(GStrv) for node_path in arm_load_dtb
Posted by Alex Bennée 3 weeks, 6 days ago
This is potentially more of a bike-shed case as node_path will persist
until the end of the function.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/arm/boot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 749f2d08341..f9d0bc7011e 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -520,11 +520,11 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
                  ARMCPU *cpu)
 {
     g_autofree void *fdt = NULL;
+    g_auto(GStrv) node_path = NULL;
     int size, rc, n = 0;
     uint32_t acells, scells;
     unsigned int i;
     hwaddr mem_base, mem_len;
-    char **node_path;
     Error *err = NULL;
 
     if (binfo->dtb_filename) {
@@ -586,7 +586,6 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         }
         n++;
     }
-    g_strfreev(node_path);
 
     /*
      * We drop all the memory nodes which correspond to empty NUMA nodes
-- 
2.47.2


Re: [PATCH 3/4] hw/arm: use g_auto(GStrv) for node_path in arm_load_dtb
Posted by Richard Henderson 3 weeks, 4 days ago
On 9/1/25 14:53, Alex Bennée wrote:
> This is potentially more of a bike-shed case as node_path will persist
> until the end of the function.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/arm/boot.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

I think you should add blocking with this.

   {
     g_auto(GStrv) node_path = qemu_fdt_node_unit_path(...)
     if (err) {
        ...
     }
     while (node_path[n]) {
        ...
     }
   }


r~

Re: [PATCH 3/4] hw/arm: use g_auto(GStrv) for node_path in arm_load_dtb
Posted by Manos Pitsidianakis 3 weeks, 6 days ago
On Mon, Sep 1, 2025 at 3:54 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> This is potentially more of a bike-shed case as node_path will persist
> until the end of the function.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  hw/arm/boot.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 749f2d08341..f9d0bc7011e 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -520,11 +520,11 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>                   ARMCPU *cpu)
>  {
>      g_autofree void *fdt = NULL;
> +    g_auto(GStrv) node_path = NULL;
>      int size, rc, n = 0;
>      uint32_t acells, scells;
>      unsigned int i;
>      hwaddr mem_base, mem_len;
> -    char **node_path;
>      Error *err = NULL;
>
>      if (binfo->dtb_filename) {
> @@ -586,7 +586,6 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>          }
>          n++;
>      }
> -    g_strfreev(node_path);
>
>      /*
>       * We drop all the memory nodes which correspond to empty NUMA nodes
> --
> 2.47.2
>
>