[PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()

[PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()

[Markus Armbruster]

Reported-by: Bernhard Beschow <shentey@gmail.com>
Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/qemu-pixman.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
index e46c6232cf..aea09755b9 100644
--- a/ui/qemu-pixman.c
+++ b/ui/qemu-pixman.c
@@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
     Error *err = NULL;
 
     qemu_win32_map_free(ptr, handle, &err);
-    error_report_err(err);
+    if (err) {
+        error_report_err(err);
+    }
 #else
     qemu_memfd_free(ptr, size, handle);
 #endif
-- 
2.49.0

Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()

[Daniel P. Berrangé]

On Mon, Oct 13, 2025 at 01:21:02PM +0200, Markus Armbruster wrote:
> Reported-by: Bernhard Beschow <shentey@gmail.com>
> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/qemu-pixman.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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] ui/pixman: Fix crash in qemu_pixman_shareable_free()

[Philippe Mathieu-Daudé]

On 13/10/25 13:21, Markus Armbruster wrote:
> Reported-by: Bernhard Beschow <shentey@gmail.com>
> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   ui/qemu-pixman.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
> index e46c6232cf..aea09755b9 100644
> --- a/ui/qemu-pixman.c
> +++ b/ui/qemu-pixman.c
> @@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>       Error *err = NULL;
>   
>       qemu_win32_map_free(ptr, handle, &err);

Slightly better if qemu_win32_map_free(), as per commit e3fe3988d78
("error: Document Error API usage rules") recommended style:

+ *  We recommend
+ *   • bool-valued functions return true on success / false on failure,
+ *   • pointer-valued functions return non-null / null pointer, and
+ *   • integer-valued functions return non-negative / negative.

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> -    error_report_err(err);
> +    if (err) {
> +        error_report_err(err);
> +    }
>   #else
>       qemu_memfd_free(ptr, size, handle);
>   #endif

Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()

[Bernhard Beschow]

Am 13. Oktober 2025 14:11:32 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 13/10/25 13:21, Markus Armbruster wrote:
>> Reported-by: Bernhard Beschow <shentey@gmail.com>
>> Fixes: b296b29d3414 (ui/pixman: Consistent error handling in qemu_pixman_shareable_free())
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   ui/qemu-pixman.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
>> index e46c6232cf..aea09755b9 100644
>> --- a/ui/qemu-pixman.c
>> +++ b/ui/qemu-pixman.c
>> @@ -291,7 +291,9 @@ qemu_pixman_shareable_free(qemu_pixman_shareable handle,
>>       Error *err = NULL;
>>         qemu_win32_map_free(ptr, handle, &err);
>
>Slightly better if qemu_win32_map_free(), as per commit e3fe3988d78
>("error: Document Error API usage rules") recommended style:
>
>+ *  We recommend
>+ *   • bool-valued functions return true on success / false on failure,
>+ *   • pointer-valued functions return non-null / null pointer, and
>+ *   • integer-valued functions return non-negative / negative.

Now I see: Since qemu_win32_map_free() returns void, the pattern is to check for the error pointer. Could be mentioned in the commit message for educational porposes.

Anyhow,
Tested-by: Bernhard Beschow
Reviewed-by: Bernhard Beschow

>
>Anyhow,
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> -    error_report_err(err);
>> +    if (err) {
>> +        error_report_err(err);
>> +    }
>>   #else
>>       qemu_memfd_free(ptr, size, handle);
>>   #endif
>