qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] curses Coverity fixes
@ 2016-08-11 14:23 Peter Maydell
  2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Gerd Hoffmann

This patchset fixes a couple of issues found by Coverity
in the curses ui frontend.

thanks
-- PMM


Peter Maydell (2):
  ui/curses.c: Ensure we don't read off the end of curses2qemu array
  ui/curses.c: Clean up nextchr logic

 ui/curses.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

-- 
2.7.4

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

* [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array
  2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell
@ 2016-08-11 14:23 ` Peter Maydell
  2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell
  2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Gerd Hoffmann

Coverity spots that there is no bounds check before we
access the curses2qemu[] array.  Add one, bringing this
code path into line with the one that looks up entries
in curses2keysym[].

In theory getch() shouldn't return out of range keycodes,
but it's better not to assume this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/curses.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ui/curses.c b/ui/curses.c
index b475589..f1f886c 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -317,7 +317,10 @@ static void curses_refresh(DisplayChangeListener *dcl)
                 qemu_input_event_send_key_delay(0);
             }
         } else {
-            keysym = curses2qemu[chr];
+            keysym = -1;
+            if (chr < CURSES_KEYS) {
+                keysym = curses2qemu[chr];
+            }
             if (keysym == -1)
                 keysym = chr;
 
-- 
2.7.4

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

* [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic
  2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell
  2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell
@ 2016-08-11 14:23 ` Peter Maydell
  2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2016-08-11 14:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches, Gerd Hoffmann

Coverity identifies that at the top of the while(1) loop
in curses_refresh() the variable nextchr is always ERR,
and so the else case of the first if() is dead code.
Remove this dead code, and narrow the scope of the
nextchr variable to the place where it's used.

(This confused logic has been present since the curses
code was added to QEMU in 2008.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/curses.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index f1f886c..d06f724 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -181,7 +181,7 @@ static kbd_layout_t *kbd_layout = NULL;
 
 static void curses_refresh(DisplayChangeListener *dcl)
 {
-    int chr, nextchr, keysym, keycode, keycode_alt;
+    int chr, keysym, keycode, keycode_alt;
 
     curses_winch_check();
 
@@ -195,15 +195,9 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
     graphic_hw_text_update(NULL, screen);
 
-    nextchr = ERR;
     while (1) {
         /* while there are any pending key strokes to process */
-        if (nextchr == ERR)
-            chr = getch();
-        else {
-            chr = nextchr;
-            nextchr = ERR;
-        }
+        chr = getch();
 
         if (chr == ERR)
             break;
@@ -224,13 +218,12 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
         /* alt key */
         if (keycode == 1) {
-            nextchr = getch();
+            int nextchr = getch();
 
             if (nextchr != ERR) {
                 chr = nextchr;
                 keycode_alt = ALT;
-                keycode = curses2keycode[nextchr];
-                nextchr = ERR;
+                keycode = curses2keycode[chr];
 
                 if (keycode != -1) {
                     keycode |= ALT;
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH 0/2] curses Coverity fixes
  2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell
  2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell
  2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell
@ 2016-09-07 11:39 ` Gerd Hoffmann
  2 siblings, 0 replies; 4+ messages in thread
From: Gerd Hoffmann @ 2016-09-07 11:39 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, patches

On Do, 2016-08-11 at 15:23 +0100, Peter Maydell wrote:
> This patchset fixes a couple of issues found by Coverity
> in the curses ui frontend.

Added to ui queue.

thanks,
  Gerd

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

end of thread, other threads:[~2016-09-07 11:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-11 14:23 [Qemu-devel] [PATCH 0/2] curses Coverity fixes Peter Maydell
2016-08-11 14:23 ` [Qemu-devel] [PATCH 1/2] ui/curses.c: Ensure we don't read off the end of curses2qemu array Peter Maydell
2016-08-11 14:23 ` [Qemu-devel] [PATCH 2/2] ui/curses.c: Clean up nextchr logic Peter Maydell
2016-09-07 11:39 ` [Qemu-devel] [PATCH 0/2] curses Coverity fixes Gerd Hoffmann

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