Re: [PATCH] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
On 13/06/2025 16:07, Daniel P. Berrangé wrote:
> On Fri, Jun 13, 2025 at 12:40:02PM +0800, Li Zhijian via wrote:
>> This leak was detected by the valgrind.
>>
>> The crs_range_merge() function unconditionally allocated a GPtrArray
>> 'even when range->len was zero, causing an early return without freeing
>> the allocated array. This resulted in a memory leak when an empty range
>> was processed.
>>
>> Fix this by moving the GPtrArray allocation after the empty range check,
>> ensuring memory is only allocated when actually needed.
>>
>> Signed-off-by: Li Zhijian
>> ---
>> hw/acpi/aml-build.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index f8f93a9f66c8..cf1999880119 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -160,7 +160,7 @@ void crs_replace_with_free_ranges(GPtrArray *ranges,
>>*/
>> static void crs_range_merge(GPtrArray *range)
>> {
>> -GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free);
>> +GPtrArray *tmp;
>
> IMHO it would be better to change this to
>
>g_autoptr(GPtrArray) tmp = g_ptr
>
>
> and remove the existing manual g_ptr_array_free call. This guarantees
> it will always be released in every code path.
It sounds good to me, I will update it soon.
Thank
>
>> CrsRangeEntry *entry;
>> uint64_t range_base, range_limit;
>> int i;
>> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
>> return;
>> }
>>
>> +tmp = g_ptr_array_new_with_free_func(crs_range_free);
>> g_ptr_array_sort(range, crs_range_compare);
>>
>> entry = g_ptr_array_index(range, 0);
>> --
>> 2.47.0
>>
>>
>
> With regards,
> Daniel
Re: [PATCH] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
On Fri, Jun 13, 2025 at 12:40:02PM +0800, Li Zhijian via wrote:
> This leak was detected by the valgrind.
>
> The crs_range_merge() function unconditionally allocated a GPtrArray
> 'even when range->len was zero, causing an early return without freeing
> the allocated array. This resulted in a memory leak when an empty range
> was processed.
>
> Fix this by moving the GPtrArray allocation after the empty range check,
> ensuring memory is only allocated when actually needed.
>
> Signed-off-by: Li Zhijian
> ---
> hw/acpi/aml-build.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f8f93a9f66c8..cf1999880119 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -160,7 +160,7 @@ void crs_replace_with_free_ranges(GPtrArray *ranges,
> */
> static void crs_range_merge(GPtrArray *range)
> {
> -GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free);
> +GPtrArray *tmp;
IMHO it would be better to change this to
g_autoptr(GPtrArray) tmp = g_ptr
and remove the existing manual g_ptr_array_free call. This guarantees
it will always be released in every code path.
> CrsRangeEntry *entry;
> uint64_t range_base, range_limit;
> int i;
> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
> return;
> }
>
> +tmp = g_ptr_array_new_with_free_func(crs_range_free);
> g_ptr_array_sort(range, crs_range_compare);
>
> entry = g_ptr_array_index(range, 0);
> --
> 2.47.0
>
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
> On 13 Jun 2025, at 10:10 AM, Li Zhijian wrote:
>
> This leak was detected by the valgrind.
>
> The crs_range_merge() function unconditionally allocated a GPtrArray
> 'even when range->len was zero, causing an early return without freeing
> the allocated array. This resulted in a memory leak when an empty range
> was processed.
>
> Fix this by moving the GPtrArray allocation after the empty range check,
> ensuring memory is only allocated when actually needed.
Thanks for the fix.
>
> Signed-off-by: Li Zhijian
Reviewed-by: Ani Sinha
> ---
> hw/acpi/aml-build.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f8f93a9f66c8..cf1999880119 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -160,7 +160,7 @@ void crs_replace_with_free_ranges(GPtrArray *ranges,
> */
> static void crs_range_merge(GPtrArray *range)
> {
> -GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free);
> +GPtrArray *tmp;
> CrsRangeEntry *entry;
> uint64_t range_base, range_limit;
> int i;
> @@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
> return;
> }
>
> +tmp = g_ptr_array_new_with_free_func(crs_range_free);
> g_ptr_array_sort(range, crs_range_compare);
>
> entry = g_ptr_array_index(range, 0);
> --
> 2.47.0
>
[PATCH] hw/acpi: Fix GPtrArray memory leak in crs_range_merge
This leak was detected by the valgrind.
The crs_range_merge() function unconditionally allocated a GPtrArray
'even when range->len was zero, causing an early return without freeing
the allocated array. This resulted in a memory leak when an empty range
was processed.
Fix this by moving the GPtrArray allocation after the empty range check,
ensuring memory is only allocated when actually needed.
Signed-off-by: Li Zhijian
---
hw/acpi/aml-build.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f8f93a9f66c8..cf1999880119 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -160,7 +160,7 @@ void crs_replace_with_free_ranges(GPtrArray *ranges,
*/
static void crs_range_merge(GPtrArray *range)
{
-GPtrArray *tmp = g_ptr_array_new_with_free_func(crs_range_free);
+GPtrArray *tmp;
CrsRangeEntry *entry;
uint64_t range_base, range_limit;
int i;
@@ -169,6 +169,7 @@ static void crs_range_merge(GPtrArray *range)
return;
}
+tmp = g_ptr_array_new_with_free_func(crs_range_free);
g_ptr_array_sort(range, crs_range_compare);
entry = g_ptr_array_index(range, 0);
--
2.47.0
