[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 18/37] acpi-build: fix array leak
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH 18/37] acpi-build: fix array leak |
Date: |
Thu, 21 Jul 2016 17:52:06 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 07/19/2016 11:54 AM, address@hidden wrote:
From: Marc-André Lureau <address@hidden>
The free_ranges array is used as a temporary pointer array, the segment
should still be freed,
Right. If I understand, this is the leak.
however, it shouldn't free the elements themself.
And it didn't, right? otherwise it would not work since these ranges
are used later.
Signed-off-by: Marc-André Lureau <address@hidden>
---
hw/i386/acpi-build.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index fbba461..f4ba3a4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -761,7 +761,7 @@ static gint crs_range_compare(gconstpointer a,
gconstpointer b)
static void crs_replace_with_free_ranges(GPtrArray *ranges,
uint64_t start, uint64_t end)
{
- GPtrArray *free_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ GPtrArray *free_ranges = g_ptr_array_new();
Indeed, we are not going to free the ranges in this array, adding the
GDestroyNotify
here is not needed.
uint64_t free_base = start;
int i;
@@ -785,7 +785,7 @@ static void crs_replace_with_free_ranges(GPtrArray *ranges,
g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
}
- g_ptr_array_free(free_ranges, false);
+ g_ptr_array_free(free_ranges, true);
This *is* scary since "true" means delete everything, but looking at
documentation:
"If array contents point to dynamically-allocated memory,
they should be freed separately if free_seg is TRUE and
no GDestroyNotify function has been set for array."
So your approach should work.
I think I understand the leak. Previous approach deleted the GArray wrapper,
preserved the pointers (which we need), but also the inner array which we don't.
One question: how did you test that it still works :) ?
Did you run something like -device pxb,id=pxb,bus_nr=0x80,bus=pci.0 -device
e1000,bus=pxb and see the device
e100 device gets the required resources?
Thanks,
Marcel
}
/*
- Re: [Qemu-devel] [PATCH 12/37] tests: fix leak in test-string-input-visitor, (continued)
- [Qemu-devel] [PATCH 13/37] portio: keep references on portio, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 15/37] pc: simplify passing qemu_irq, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 14/37] numa: do not leak NumaOptions, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 16/37] pc: don't leak a20_line, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 17/37] machine: use class base init generated name, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 18/37] acpi-build: fix array leak, marcandre . lureau, 2016/07/19
- Re: [Qemu-devel] [PATCH 18/37] acpi-build: fix array leak,
Marcel Apfelbaum <=
[Qemu-devel] [PATCH 19/37] char: disconnect peer when qemu_chr_free(), marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 20/37] char: free MuxDriver when closing, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 21/37] tests: fix qom-test leaks, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 22/37] pc: free i8259, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 23/37] pci-bus: do not allocate and leak bsel, marcandre . lureau, 2016/07/19