On 07/19/2016 11:54 AM, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau <marcandre.lur...@redhat.com>
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 <marcandre.lur...@redhat.com>
---
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
}
/*
