| Message ID | 20240822162323.706382-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-9.2] hw/arm/sbsa-ref: Don't leak string in sbsa_fdt_add_gic_node() | expand |
On 22/8/24 18:23, Peter Maydell wrote: > In sbsa_fdt_add_gic_node() we g_strdup_printf() two nodename > strings, but only free one. > > Since the string is actually entirely constant and we don't > make any use of printf's format-string operations, we can > drop the g_strdup_printf() use entirely. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > A small once-only leak, so this is 9.2 material. Spotted > with clang leak-sanitizer. > > hw/arm/sbsa-ref.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 22/8/24 18:23, Peter Maydell wrote: > In sbsa_fdt_add_gic_node() we g_strdup_printf() two nodename > strings, but only free one. > > Since the string is actually entirely constant and we don't > make any use of printf's format-string operations, we can > drop the g_strdup_printf() use entirely. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > A small once-only leak, so this is 9.2 material. Spotted > with clang leak-sanitizer. > > hw/arm/sbsa-ref.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index ae37a923015..5cd8cd705be 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -164,23 +164,20 @@ static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) > > static void sbsa_fdt_add_gic_node(SBSAMachineState *sms) > { > - char *nodename; > + const char *intc_nodename = "/intc"; > + const char *its_nodename = "/intc/its"; Should we use static qualifiers?
On 8/23/24 16:42, Philippe Mathieu-Daudé wrote: >> static void sbsa_fdt_add_gic_node(SBSAMachineState *sms) >> { >> - char *nodename; >> + const char *intc_nodename = "/intc"; >> + const char *its_nodename = "/intc/its"; > > Should we use static qualifiers?' No. The real object is the string literal. The local variable simply allows multiple references within the function. r~
On 8/23/24 2:23 AM, Peter Maydell wrote: > In sbsa_fdt_add_gic_node() we g_strdup_printf() two nodename > strings, but only free one. > > Since the string is actually entirely constant and we don't > make any use of printf's format-string operations, we can > drop the g_strdup_printf() use entirely. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > A small once-only leak, so this is 9.2 material. Spotted > with clang leak-sanitizer. > > hw/arm/sbsa-ref.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > Reviewed-by: Gavin Shan <gshan@redhat.com>
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index ae37a923015..5cd8cd705be 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -164,23 +164,20 @@ static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) static void sbsa_fdt_add_gic_node(SBSAMachineState *sms) { - char *nodename; + const char *intc_nodename = "/intc"; + const char *its_nodename = "/intc/its"; - nodename = g_strdup_printf("/intc"); - qemu_fdt_add_subnode(sms->fdt, nodename); - qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg", + qemu_fdt_add_subnode(sms->fdt, intc_nodename); + qemu_fdt_setprop_sized_cells(sms->fdt, intc_nodename, "reg", 2, sbsa_ref_memmap[SBSA_GIC_DIST].base, 2, sbsa_ref_memmap[SBSA_GIC_DIST].size, 2, sbsa_ref_memmap[SBSA_GIC_REDIST].base, 2, sbsa_ref_memmap[SBSA_GIC_REDIST].size); - nodename = g_strdup_printf("/intc/its"); - qemu_fdt_add_subnode(sms->fdt, nodename); - qemu_fdt_setprop_sized_cells(sms->fdt, nodename, "reg", + qemu_fdt_add_subnode(sms->fdt, its_nodename); + qemu_fdt_setprop_sized_cells(sms->fdt, its_nodename, "reg", 2, sbsa_ref_memmap[SBSA_GIC_ITS].base, 2, sbsa_ref_memmap[SBSA_GIC_ITS].size); - - g_free(nodename); } /*
In sbsa_fdt_add_gic_node() we g_strdup_printf() two nodename strings, but only free one. Since the string is actually entirely constant and we don't make any use of printf's format-string operations, we can drop the g_strdup_printf() use entirely. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- A small once-only leak, so this is 9.2 material. Spotted with clang leak-sanitizer. hw/arm/sbsa-ref.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)