qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
@ 2025-10-13 11:21 Markus Armbruster
  2025-10-13 11:51 ` Daniel P. Berrangé
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Markus Armbruster @ 2025-10-13 11:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, Bernhard Beschow

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-13 11:21 [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free() Markus Armbruster
@ 2025-10-13 11:51 ` Daniel P. Berrangé
  2025-10-13 14:11 ` Philippe Mathieu-Daudé
  2025-10-20 10:30 ` Bernhard Beschow
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2025-10-13 11:51 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, marcandre.lureau, Bernhard Beschow

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 :|



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-13 11:21 [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free() Markus Armbruster
  2025-10-13 11:51 ` Daniel P. Berrangé
@ 2025-10-13 14:11 ` Philippe Mathieu-Daudé
  2025-10-13 19:12   ` Bernhard Beschow
  2025-10-14 12:52   ` Markus Armbruster
  2025-10-20 10:30 ` Bernhard Beschow
  2 siblings, 2 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-13 14:11 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: marcandre.lureau, Bernhard Beschow

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-13 14:11 ` Philippe Mathieu-Daudé
@ 2025-10-13 19:12   ` Bernhard Beschow
  2025-10-14 12:52   ` Markus Armbruster
  1 sibling, 0 replies; 10+ messages in thread
From: Bernhard Beschow @ 2025-10-13 19:12 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Markus Armbruster, qemu-devel
  Cc: marcandre.lureau



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
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-13 14:11 ` Philippe Mathieu-Daudé
  2025-10-13 19:12   ` Bernhard Beschow
@ 2025-10-14 12:52   ` Markus Armbruster
  1 sibling, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2025-10-14 12:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, marcandre.lureau, Bernhard Beschow

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> 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.

I went fo a minimal fix.

I now see that this is the only use of qemu_win32_map_free().  Changing
it to return bool would've been okay.

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

Thanks!

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-13 11:21 [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free() Markus Armbruster
  2025-10-13 11:51 ` Daniel P. Berrangé
  2025-10-13 14:11 ` Philippe Mathieu-Daudé
@ 2025-10-20 10:30 ` Bernhard Beschow
  2025-10-20 12:32   ` Markus Armbruster
  2 siblings, 1 reply; 10+ messages in thread
From: Bernhard Beschow @ 2025-10-20 10:30 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: marcandre.lureau



Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>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

Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?

Best regards,
Bernhard


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-20 10:30 ` Bernhard Beschow
@ 2025-10-20 12:32   ` Markus Armbruster
  2025-10-20 17:27     ` Bernhard Beschow
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2025-10-20 12:32 UTC (permalink / raw)
  To: Bernhard Beschow; +Cc: qemu-devel, marcandre.lureau

Bernhard Beschow <shentey@gmail.com> writes:

> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>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
>
> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?

I can do it.

You provided

    Tested-by: Bernhard Beschow
    Reviewed-by: Bernhard Beschow

without an e-mail address.  Intentional?  If not, should I use
<shentey@gmail.com>?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-20 12:32   ` Markus Armbruster
@ 2025-10-20 17:27     ` Bernhard Beschow
  2025-10-21  9:50       ` Markus Armbruster
  0 siblings, 1 reply; 10+ messages in thread
From: Bernhard Beschow @ 2025-10-20 17:27 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, marcandre.lureau



Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Bernhard Beschow <shentey@gmail.com> writes:
>
>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>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
>>
>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>
>I can do it.
>
>You provided
>
>    Tested-by: Bernhard Beschow
>    Reviewed-by: Bernhard Beschow
>
>without an e-mail address.  Intentional?

Oh, that wasn't intentional.

> If not, should I use <shentey@gmail.com>?

Yes, please.

Best regards,
Bernhard


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-20 17:27     ` Bernhard Beschow
@ 2025-10-21  9:50       ` Markus Armbruster
  2025-10-21 17:22         ` Bernhard Beschow
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2025-10-21  9:50 UTC (permalink / raw)
  To: Bernhard Beschow; +Cc: qemu-devel, marcandre.lureau

Bernhard Beschow <shentey@gmail.com> writes:

> Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>Bernhard Beschow <shentey@gmail.com> writes:
>>
>>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>>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
>>>
>>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>>
>>I can do it.
>>
>>You provided
>>
>>    Tested-by: Bernhard Beschow
>>    Reviewed-by: Bernhard Beschow
>>
>>without an e-mail address.  Intentional?
>
> Oh, that wasn't intentional.
>
>> If not, should I use <shentey@gmail.com>?
>
> Yes, please.

Done, PR sent.  Thank you!



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free()
  2025-10-21  9:50       ` Markus Armbruster
@ 2025-10-21 17:22         ` Bernhard Beschow
  0 siblings, 0 replies; 10+ messages in thread
From: Bernhard Beschow @ 2025-10-21 17:22 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, marcandre.lureau



Am 21. Oktober 2025 09:50:32 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Bernhard Beschow <shentey@gmail.com> writes:
>
>> Am 20. Oktober 2025 12:32:33 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>Bernhard Beschow <shentey@gmail.com> writes:
>>>
>>>> Am 13. Oktober 2025 11:21:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>>>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
>>>>
>>>> Thanks for the quick fix, Markus! The patch is reviewed and I've been testing it for a week. Who is going to merge it?
>>>
>>>I can do it.
>>>
>>>You provided
>>>
>>>    Tested-by: Bernhard Beschow
>>>    Reviewed-by: Bernhard Beschow
>>>
>>>without an e-mail address.  Intentional?
>>
>> Oh, that wasn't intentional.
>>
>>> If not, should I use <shentey@gmail.com>?
>>
>> Yes, please.
>
>Done, PR sent.  Thank you!

Nice. Thanks!


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-10-21 17:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 11:21 [PATCH] ui/pixman: Fix crash in qemu_pixman_shareable_free() Markus Armbruster
2025-10-13 11:51 ` Daniel P. Berrangé
2025-10-13 14:11 ` Philippe Mathieu-Daudé
2025-10-13 19:12   ` Bernhard Beschow
2025-10-14 12:52   ` Markus Armbruster
2025-10-20 10:30 ` Bernhard Beschow
2025-10-20 12:32   ` Markus Armbruster
2025-10-20 17:27     ` Bernhard Beschow
2025-10-21  9:50       ` Markus Armbruster
2025-10-21 17:22         ` Bernhard Beschow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).