Re: [PATCH v2] hw/loongarch/virt: Fix memory leak

[Peter Maydell]

On Wed, 8 May 2024 at 03:30, Song Gao <gaosong@loongson.cn> wrote:
>
> The char pointer 'ramName' point to a block of memory, but never free it.
> Use a small fixed-size buffer for 'ramName'.
>
> Resolves: Coverity CID 1544773
>
> Fixes: 0cf1478d6 ("hw/loongarch: Add numa support")
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
> v2:
>   - Use a small fixed-size buffer for 'ramName'.
>   - Link to V1: 
> 20240507022239.3113987-1-gaosong@loongson.cn/">https://patchew.org/QEMU/20240507022239.3113987-1-gaosong@loongson.cn/
>
>  hw/loongarch/virt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index c0999878df..ee690ad981 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -887,7 +887,7 @@ static void loongarch_init(MachineState *machine)
>      const CPUArchIdList *possible_cpus;
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      CPUState *cpu;
> -    char *ramName = NULL;
> +    char ramName[32];

Please don't use fixed-size char arrays for writing strings like this.

>      if (!cpu_model) {
>          cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
> @@ -946,7 +946,7 @@ static void loongarch_init(MachineState *machine)
>
>      for (i = 1; i < nb_numa_nodes; i++) {
>          MemoryRegion *nodemem = g_new(MemoryRegion, 1);
> -        ramName = g_strdup_printf("loongarch.node%d.ram", i);
> +        sprintf(ramName, "loongarch.node%d.ram", i);

The nicest way to fix this is to use the g_autofree mechanism
so the memory is automatically freed at the end of the block:

   g_autofree char *ramName = g_strdup_printf(...);

>          memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
>                                   offset,  numa_info[i].node_mem);
>          memory_region_add_subregion(address_space_mem, phyAddr, nodemem);

thanks
-- PMM