Re: [PATCH] hw/loongarch/virt: Fix memory leak
On 7/5/24 04:22, Song Gao wrote: The char pointer 'ramName' point to a block of memory, but never free it. Use 'g_autofree' to automatically free it. Resolves: Coverity CID 1544773 Fixes: 0cf1478d6 ("hw/loongarch: Add numa support") Signed-off-by: Song Gao --- hw/loongarch/virt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Thanks, patch queued to hw-misc tree.
Re: [PATCH] hw/loongarch/virt: Fix memory leak
On Tue, 7 May 2024 at 10:52, Michael Tokarev wrote: > > 07.05.2024 05:22, Song Gao wrote: > > > for (i = 1; i < nb_numa_nodes; i++) { > > MemoryRegion *nodemem = g_new(MemoryRegion, 1); > > -ramName = g_strdup_printf("loongarch.node%d.ram", i); > > +g_autofree char *ramName = g_strdup_printf("loongarch.node%d.ram", > > i); > > Can't this be a fixed-size buffer on stack? No, this is a really bad idea. It's a pain to audit that the array really doesn't get overwritten, and if the string we want to write changes, now we have to re-count characters to decide if we need to increase the size of the array. The memory allocation on the heap here is a tiny overhead that we only incur at startup. The g_autofree approach is much better. For this version of the patch: Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] hw/loongarch/virt: Fix memory leak
在 2024/5/7 下午5:52, Michael Tokarev 写道: 07.05.2024 05:22, Song Gao wrote: for (i = 1; i < nb_numa_nodes; i++) { MemoryRegion *nodemem = g_new(MemoryRegion, 1); - ramName = g_strdup_printf("loongarch.node%d.ram", i); + g_autofree char *ramName = g_strdup_printf("loongarch.node%d.ram", i); Can't this be a fixed-size buffer on stack? Maybe I'm old-minded, but such obviously fixed and very small allocations on the heap hurt my eyes ;) I had send v2 patch. Thanks. Song Gao /mjt
Re: [PATCH] hw/loongarch/virt: Fix memory leak
07.05.2024 05:22, Song Gao wrote: for (i = 1; i < nb_numa_nodes; i++) { MemoryRegion *nodemem = g_new(MemoryRegion, 1); -ramName = g_strdup_printf("loongarch.node%d.ram", i); +g_autofree char *ramName = g_strdup_printf("loongarch.node%d.ram", i); Can't this be a fixed-size buffer on stack? Maybe I'm old-minded, but such obviously fixed and very small allocations on the heap hurt my eyes ;) /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
Re: [PATCH] hw/loongarch/virt: Fix memory leak
On 7/5/24 04:22, Song Gao wrote: The char pointer 'ramName' point to a block of memory, but never free it. Use 'g_autofree' to automatically free it. Resolves: Coverity CID 1544773 Fixes: 0cf1478d6 ("hw/loongarch: Add numa support") Signed-off-by: Song Gao --- hw/loongarch/virt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[PATCH] hw/loongarch/virt: Fix memory leak
The char pointer 'ramName' point to a block of memory, but never free it. Use 'g_autofree' to automatically free it. Resolves: Coverity CID 1544773 Fixes: 0cf1478d6 ("hw/loongarch: Add numa support") Signed-off-by: Song Gao --- hw/loongarch/virt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index c0999878df..ea5100be6d 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -887,7 +887,6 @@ static void loongarch_init(MachineState *machine) const CPUArchIdList *possible_cpus; MachineClass *mc = MACHINE_GET_CLASS(machine); CPUState *cpu; -char *ramName = NULL; if (!cpu_model) { cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); @@ -946,7 +945,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); +g_autofree char *ramName = g_strdup_printf("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); -- 2.25.1