diff mbox series

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

Message ID 20240508023031.3127531-1-gaosong@loongson.cn
State New
Headers show
Series [v2] hw/loongarch/virt: Fix memory leak | expand

Commit Message

Song Gao May 8, 2024, 2:30 a.m. UTC
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: https://patchew.org/QEMU/20240507022239.3113987-1-gaosong@loongson.cn/ 

 hw/loongarch/virt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Maydell May 8, 2024, 1:02 p.m. UTC | #1
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: 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
diff mbox series

Patch

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];
 
     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);
         memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
                                  offset,  numa_info[i].node_mem);
         memory_region_add_subregion(address_space_mem, phyAddr, nodemem);