* [PATCH] hw/loongarch/virt: Fix memory leak
@ 2024-05-07 2:22 Song Gao
2024-05-07 7:28 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Song Gao @ 2024-05-07 2:22 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, qemu-stable, richard.henderson, zhaotianrui
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 <gaosong@loongson.cn>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Fix memory leak
2024-05-07 2:22 [PATCH] hw/loongarch/virt: Fix memory leak Song Gao
@ 2024-05-07 7:28 ` Philippe Mathieu-Daudé
2024-05-07 9:52 ` Michael Tokarev
2024-05-08 21:17 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-07 7:28 UTC (permalink / raw)
To: Song Gao, peter.maydell
Cc: qemu-devel, qemu-stable, richard.henderson, zhaotianrui
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 <gaosong@loongson.cn>
> ---
> hw/loongarch/virt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Fix memory leak
2024-05-07 2:22 [PATCH] hw/loongarch/virt: Fix memory leak Song Gao
2024-05-07 7:28 ` Philippe Mathieu-Daudé
@ 2024-05-07 9:52 ` Michael Tokarev
2024-05-08 2:49 ` gaosong
2024-05-08 13:05 ` Peter Maydell
2024-05-08 21:17 ` Philippe Mathieu-Daudé
2 siblings, 2 replies; 6+ messages in thread
From: Michael Tokarev @ 2024-05-07 9:52 UTC (permalink / raw)
To: Song Gao, peter.maydell
Cc: qemu-devel, qemu-stable, richard.henderson, zhaotianrui
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Fix memory leak
2024-05-07 9:52 ` Michael Tokarev
@ 2024-05-08 2:49 ` gaosong
2024-05-08 13:05 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: gaosong @ 2024-05-08 2:49 UTC (permalink / raw)
To: Michael Tokarev
Cc: peter.maydell, qemu-devel, qemu-stable, richard.henderson, zhaotianrui
在 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Fix memory leak
2024-05-07 9:52 ` Michael Tokarev
2024-05-08 2:49 ` gaosong
@ 2024-05-08 13:05 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2024-05-08 13:05 UTC (permalink / raw)
To: Michael Tokarev
Cc: Song Gao, qemu-devel, qemu-stable, richard.henderson, zhaotianrui
On Tue, 7 May 2024 at 10:52, Michael Tokarev <mjt@tls.msk.ru> 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 <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Fix memory leak
2024-05-07 2:22 [PATCH] hw/loongarch/virt: Fix memory leak Song Gao
2024-05-07 7:28 ` Philippe Mathieu-Daudé
2024-05-07 9:52 ` Michael Tokarev
@ 2024-05-08 21:17 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 21:17 UTC (permalink / raw)
To: Song Gao, peter.maydell
Cc: qemu-devel, qemu-stable, richard.henderson, zhaotianrui
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 <gaosong@loongson.cn>
> ---
> hw/loongarch/virt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Thanks, patch queued to hw-misc tree.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-08 21:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-07 2:22 [PATCH] hw/loongarch/virt: Fix memory leak Song Gao
2024-05-07 7:28 ` Philippe Mathieu-Daudé
2024-05-07 9:52 ` Michael Tokarev
2024-05-08 2:49 ` gaosong
2024-05-08 13:05 ` Peter Maydell
2024-05-08 21:17 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.