* [PATCH] ui/curses: Fix infinite loop on windows
@ 2025-04-03 1:07 William Hu via
2025-04-08 14:08 ` Stefan Hajnoczi
2025-04-08 19:01 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: William Hu via @ 2025-04-03 1:07 UTC (permalink / raw)
To: qemu-devel@nongnu.org
From a42046272f0544dd18ed58661e53ea17d1584c2c Mon Sep 17 00:00:00 2001
From: William Hu <purplearmadillo77@proton.me>
Date: Wed, 2 Apr 2025 12:00:00 -0400
Subject: [PATCH] ui/curses: Fix infinite loop on windows
Replace -1 comparisons for wint_t with WEOF to fix infinite loop caused by a
65535 == -1 comparison.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2905
Signed-off-by: William Hu <purplearmadillo77@proton.me>
---
ui/curses.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/ui/curses.c b/ui/curses.c
index a39aee8762..3f5c5adf78 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -265,7 +265,12 @@ static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
static void curses_refresh(DisplayChangeListener *dcl)
{
- int chr, keysym, keycode, keycode_alt;
+ /*
+ * DO NOT MAKE chr AN INT:
+ * Causes silent conversion errors on Windows where wint_t is unsigned short.
+ */
+ wint_t chr = 0;
+ int keysym, keycode, keycode_alt;
enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
curses_winch_check();
@@ -284,8 +289,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
/* while there are any pending key strokes to process */
chr = console_getch(&maybe_keycode);
- if (chr == -1)
+ if (chr == WEOF) {
break;
+ }
#ifdef KEY_RESIZE
/* this shouldn't occur when we use a custom SIGWINCH handler */
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ui/curses: Fix infinite loop on windows
2025-04-03 1:07 [PATCH] ui/curses: Fix infinite loop on windows William Hu via
@ 2025-04-08 14:08 ` Stefan Hajnoczi
2025-04-08 22:28 ` Philippe Mathieu-Daudé
2025-04-08 19:01 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2025-04-08 14:08 UTC (permalink / raw)
To: William Hu; +Cc: qemu-devel@nongnu.org, kraxel, Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]
On Thu, Apr 03, 2025 at 01:07:56AM +0000, William Hu via wrote:
> >From a42046272f0544dd18ed58661e53ea17d1584c2c Mon Sep 17 00:00:00 2001
> From: William Hu <purplearmadillo77@proton.me>
> Date: Wed, 2 Apr 2025 12:00:00 -0400
> Subject: [PATCH] ui/curses: Fix infinite loop on windows
>
> Replace -1 comparisons for wint_t with WEOF to fix infinite loop caused by a
> 65535 == -1 comparison.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2905
> Signed-off-by: William Hu <purplearmadillo77@proton.me>
> ---
> ui/curses.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
I have CCed Gerd Hoffmann (git-shortlog(1) shows he is the most frequent
committer to this source file) and Marc-André Lureau (ui/ maintainer
according to the ./MAINTAINERS file) so they can also review your patch.
>
> diff --git a/ui/curses.c b/ui/curses.c
> index a39aee8762..3f5c5adf78 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -265,7 +265,12 @@ static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
>
> static void curses_refresh(DisplayChangeListener *dcl)
> {
> - int chr, keysym, keycode, keycode_alt;
> + /*
> + * DO NOT MAKE chr AN INT:
> + * Causes silent conversion errors on Windows where wint_t is unsigned short.
> + */
> + wint_t chr = 0;
> + int keysym, keycode, keycode_alt;
> enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
>
> curses_winch_check();
> @@ -284,8 +289,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
> /* while there are any pending key strokes to process */
> chr = console_getch(&maybe_keycode);
>
> - if (chr == -1)
> + if (chr == WEOF) {
> break;
> + }
Further below there appears to be another instance of the same bug:
/* alt or esc key */
if (keycode == 1) {
enum maybe_keycode next_maybe_keycode = CURSES_KEYCODE;
int nextchr = console_getch(&next_maybe_keycode);
if (nextchr != -1) {
^^^^^^^^^^^^^
>
> #ifdef KEY_RESIZE
> /* this shouldn't occur when we use a custom SIGWINCH handler */
> --
> 2.47.0
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ui/curses: Fix infinite loop on windows
2025-04-08 14:08 ` Stefan Hajnoczi
@ 2025-04-08 22:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-08 22:28 UTC (permalink / raw)
To: Stefan Hajnoczi, William Hu
Cc: qemu-devel@nongnu.org, kraxel, Marc-André Lureau
On 8/4/25 16:08, Stefan Hajnoczi wrote:
> On Thu, Apr 03, 2025 at 01:07:56AM +0000, William Hu via wrote:
>> >From a42046272f0544dd18ed58661e53ea17d1584c2c Mon Sep 17 00:00:00 2001
>> From: William Hu <purplearmadillo77@proton.me>
>> Date: Wed, 2 Apr 2025 12:00:00 -0400
>> Subject: [PATCH] ui/curses: Fix infinite loop on windows
>>
>> Replace -1 comparisons for wint_t with WEOF to fix infinite loop caused by a
>> 65535 == -1 comparison.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2905
>> Signed-off-by: William Hu <purplearmadillo77@proton.me>
>> ---
>> ui/curses.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> I have CCed Gerd Hoffmann (git-shortlog(1) shows he is the most frequent
> committer to this source file) and Marc-André Lureau (ui/ maintainer
> according to the ./MAINTAINERS file) so they can also review your patch.
>
>>
>> diff --git a/ui/curses.c b/ui/curses.c
>> index a39aee8762..3f5c5adf78 100644
>> --- a/ui/curses.c
>> +++ b/ui/curses.c
>> @@ -265,7 +265,12 @@ static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
>>
>> static void curses_refresh(DisplayChangeListener *dcl)
>> {
>> - int chr, keysym, keycode, keycode_alt;
>> + /*
>> + * DO NOT MAKE chr AN INT:
>> + * Causes silent conversion errors on Windows where wint_t is unsigned short.
>> + */
>> + wint_t chr = 0;
>> + int keysym, keycode, keycode_alt;
>> enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
>>
>> curses_winch_check();
>> @@ -284,8 +289,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
>> /* while there are any pending key strokes to process */
>> chr = console_getch(&maybe_keycode);
>>
>> - if (chr == -1)
>> + if (chr == WEOF) {
>> break;
>> + }
>
> Further below there appears to be another instance of the same bug:
>
> /* alt or esc key */
> if (keycode == 1) {
> enum maybe_keycode next_maybe_keycode = CURSES_KEYCODE;
> int nextchr = console_getch(&next_maybe_keycode);
>
> if (nextchr != -1) {
> ^^^^^^^^^^^^^
Indeed.
The changes comes from commit 459a707eccc ("curses: support wide input")
from 2019. This isn't a blocker for the next release IMHO.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ui/curses: Fix infinite loop on windows
2025-04-03 1:07 [PATCH] ui/curses: Fix infinite loop on windows William Hu via
2025-04-08 14:08 ` Stefan Hajnoczi
@ 2025-04-08 19:01 ` Philippe Mathieu-Daudé
2025-07-21 8:35 ` [PATCH-for-10.1] " Philippe Mathieu-Daudé
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-08 19:01 UTC (permalink / raw)
To: William Hu, qemu-devel@nongnu.org
On 3/4/25 03:07, William Hu via wrote:
> From a42046272f0544dd18ed58661e53ea17d1584c2c Mon Sep 17 00:00:00 2001
> From: William Hu <purplearmadillo77@proton.me>
> Date: Wed, 2 Apr 2025 12:00:00 -0400
> Subject: [PATCH] ui/curses: Fix infinite loop on windows
>
> Replace -1 comparisons for wint_t with WEOF to fix infinite loop caused by a
> 65535 == -1 comparison.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2905
> Signed-off-by: William Hu <purplearmadillo77@proton.me>
> ---
> ui/curses.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ui/curses.c b/ui/curses.c
> index a39aee8762..3f5c5adf78 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -265,7 +265,12 @@ static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
>
> static void curses_refresh(DisplayChangeListener *dcl)
> {
> - int chr, keysym, keycode, keycode_alt;
> + /*
> + * DO NOT MAKE chr AN INT:
> + * Causes silent conversion errors on Windows where wint_t is unsigned short.
> + */
> + wint_t chr = 0;
> + int keysym, keycode, keycode_alt;
> enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
>
> curses_winch_check();
> @@ -284,8 +289,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
> /* while there are any pending key strokes to process */
> chr = console_getch(&maybe_keycode);
>
> - if (chr == -1)
> + if (chr == WEOF) {
> break;
> + }
Correct but incomplete, also missing the same check few lines below:
-- >8 --
diff --git a/ui/curses.c b/ui/curses.c
index a39aee87623..9c33de331cd 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -304,9 +304,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
/* alt or esc key */
if (keycode == 1) {
enum maybe_keycode next_maybe_keycode = CURSES_KEYCODE;
- int nextchr = console_getch(&next_maybe_keycode);
+ wint_t nextchr = console_getch(&next_maybe_keycode);
- if (nextchr != -1) {
+ if (nextchr != WEOF) {
chr = nextchr;
maybe_keycode = next_maybe_keycode;
keycode_alt = ALT;
---
With that:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH-for-10.1] ui/curses: Fix infinite loop on windows
2025-04-08 19:01 ` Philippe Mathieu-Daudé
@ 2025-07-21 8:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-21 8:35 UTC (permalink / raw)
To: William Hu, qemu-devel@nongnu.org; +Cc: Michael Tokarev, Marc-André Lureau
Ping?
On 8/4/25 21:01, Philippe Mathieu-Daudé wrote:
> On 3/4/25 03:07, William Hu via wrote:
>> From a42046272f0544dd18ed58661e53ea17d1584c2c Mon Sep 17 00:00:00 2001
>> From: William Hu <purplearmadillo77@proton.me>
>> Date: Wed, 2 Apr 2025 12:00:00 -0400
>> Subject: [PATCH] ui/curses: Fix infinite loop on windows
>>
>> Replace -1 comparisons for wint_t with WEOF to fix infinite loop
>> caused by a
>> 65535 == -1 comparison.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2905
>> Signed-off-by: William Hu <purplearmadillo77@proton.me>
>> ---
>> ui/curses.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/ui/curses.c b/ui/curses.c
>> index a39aee8762..3f5c5adf78 100644
>> --- a/ui/curses.c
>> +++ b/ui/curses.c
>> @@ -265,7 +265,12 @@ static int curses2foo(const int _curses2foo[],
>> const int _curseskey2foo[],
>> static void curses_refresh(DisplayChangeListener *dcl)
>> {
>> - int chr, keysym, keycode, keycode_alt;
>> + /*
>> + * DO NOT MAKE chr AN INT:
>> + * Causes silent conversion errors on Windows where wint_t is
>> unsigned short.
>> + */
>> + wint_t chr = 0;
>> + int keysym, keycode, keycode_alt;
>> enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
>> curses_winch_check();
>> @@ -284,8 +289,9 @@ static void curses_refresh(DisplayChangeListener
>> *dcl)
>> /* while there are any pending key strokes to process */
>> chr = console_getch(&maybe_keycode);
>> - if (chr == -1)
>> + if (chr == WEOF) {
>> break;
>> + }
>
> Correct but incomplete, also missing the same check few lines below:
>
> -- >8 --
> diff --git a/ui/curses.c b/ui/curses.c
> index a39aee87623..9c33de331cd 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -304,9 +304,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
> /* alt or esc key */
> if (keycode == 1) {
> enum maybe_keycode next_maybe_keycode = CURSES_KEYCODE;
> - int nextchr = console_getch(&next_maybe_keycode);
> + wint_t nextchr = console_getch(&next_maybe_keycode);
>
> - if (nextchr != -1) {
> + if (nextchr != WEOF) {
> chr = nextchr;
> maybe_keycode = next_maybe_keycode;
> keycode_alt = ALT;
> ---
>
> With that:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-21 8:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-03 1:07 [PATCH] ui/curses: Fix infinite loop on windows William Hu via
2025-04-08 14:08 ` Stefan Hajnoczi
2025-04-08 22:28 ` Philippe Mathieu-Daudé
2025-04-08 19:01 ` Philippe Mathieu-Daudé
2025-07-21 8:35 ` [PATCH-for-10.1] " Philippe Mathieu-Daudé
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).