[Qemu-devel] [PATCH] fix curses update

[Qemu-devel] [PATCH] fix curses update

[Bernhard Kauer]

If a terminal is resized or the VGA model issues a full refresh, curses_update()
is called, which uses mvwaddchnstr() to draw a full line of characters.  Unfortunatelly
this routine expects a null-terminated string and early aborts if a null is present
in the line. 

When booting an OS that zeros the VGA text buffer and later pokes single characters,
the console output can become unreadable.  The attached patch corrects this bug.


	Bernhard Kauer



Signed-off-by: Bernhard Kauer <kauer@tudos.org>

diff --git a/curses.c b/curses.c
index ed3165e..9bf9265 100644
--- a/curses.c
+++ b/curses.c
@@ -48,10 +48,12 @@ static int px, py, sminx, sminy, smaxx, smaxy;
 static void curses_update(DisplayState *ds, int x, int y, int w, int h)
 {
     chtype *line;
+    int i;
 
     line = ((chtype *) screen) + y * width;
     for (h += y; y < h; y ++, line += width)
-        mvwaddchnstr(screenpad, y, 0, line, width);
+      for (i = 0; i < width; i++)
+	mvwaddch(screenpad, y, i, (line[i] & 0xff) ? line[i] : ' ');
 
     pnoutrefresh(screenpad, py, px, sminy, sminx, smaxy - 1, smaxx - 1);
     refresh();

Re: [Qemu-devel] [PATCH] fix curses update

[andrzej zaborowski]

Hi,

On 20 April 2010 11:38, Bernhard Kauer <kauer@os.inf.tu-dresden.de> wrote:
> If a terminal is resized or the VGA model issues a full refresh, curses_update()
> is called, which uses mvwaddchnstr() to draw a full line of characters.  Unfortunatelly
> this routine expects a null-terminated string and early aborts if a null is present
> in the line.
>
> When booting an OS that zeros the VGA text buffer and later pokes single characters,
> the console output can become unreadable.  The attached patch corrects this bug.

I believe this issue has come up before with a similar patch but
someone checked their ncurses and they didn't see the same issue.  I
just checked and here mvwaddchnstr() does not expect a null-terminated
string either, but it skips the \0 characters.  So probably we should
replace them with spaces or something else,  I wouldn't like to
replace a single library call with 80 calls, it's better to go through
the string and replace them, maybe in console_write_ch or somewhere
else.

Cheers

Re: [Qemu-devel] [PATCH] fix curses update

[Bernhard Kauer]

Hi,

> I believe this issue has come up before with a similar patch but

well i've submitted such a patch more than two years ago.  Unfortunatelly
it got never applied, so that I have to patch my Qemu on every update...


> someone checked their ncurses and they didn't see the same issue. 
> I just checked and here mvwaddchnstr() does not expect a null-terminated
> string either, but it skips the \0 characters.

This is not conforming to the Single UNIX Specification, which states
that the string is shown "until a null chtype is encountered". See for
example:
  http://www.opengroup.org/onlinepubs/007908775/xcurses/addchstr.html 


>  So probably we should
> replace them with spaces or something else,  I wouldn't like to
> replace a single library call with 80 calls, it's better to go through
> the string and replace them, maybe in console_write_ch or somewhere
> else.

That would be a one-liner.  Should I send such a patch?


Thanks,

	Bernhard

Re: [Qemu-devel] [PATCH] fix curses update

[Anthony Liguori]

On 04/22/2010 09:08 AM, Bernhard Kauer wrote:
> Hi,
>
>    
>> I believe this issue has come up before with a similar patch but
>>      
> well i've submitted such a patch more than two years ago.  Unfortunatelly
> it got never applied, so that I have to patch my Qemu on every update...
>
>
>    
>> someone checked their ncurses and they didn't see the same issue.
>> I just checked and here mvwaddchnstr() does not expect a null-terminated
>> string either, but it skips the \0 characters.
>>      
> This is not conforming to the Single UNIX Specification, which states
> that the string is shown "until a null chtype is encountered". See for
> example:
>    http://www.opengroup.org/onlinepubs/007908775/xcurses/addchstr.html
>
>
>    
>>   So probably we should
>> replace them with spaces or something else,  I wouldn't like to
>> replace a single library call with 80 calls, it's better to go through
>> the string and replace them, maybe in console_write_ch or somewhere
>> else.
>>      
> That would be a one-liner.  Should I send such a patch?
>    

Yes.

Regards,

Anthony Liguori

> Thanks,
>
> 	Bernhard
>
>
>
>    

[Qemu-devel] [PATCH] fix curses update - v2

[Bernhard Kauer]

On Mon, May 03, 2010 at 01:06:46PM -0500, Anthony Liguori wrote:
> On 04/22/2010 09:08 AM, Bernhard Kauer wrote:
> >Hi,
> >
> >>I believe this issue has come up before with a similar patch but
> >well i've submitted such a patch more than two years ago.  Unfortunatelly
> >it got never applied, so that I have to patch my Qemu on every update...
> >
> >
> >>someone checked their ncurses and they didn't see the same issue.
> >>I just checked and here mvwaddchnstr() does not expect a null-terminated
> >>string either, but it skips the \0 characters.
> >This is not conforming to the Single UNIX Specification, which states
> >that the string is shown "until a null chtype is encountered". See for
> >example:
> >   http://www.opengroup.org/onlinepubs/007908775/xcurses/addchstr.html
> >
> >
> >>  So probably we should
> >>replace them with spaces or something else,  I wouldn't like to
> >>replace a single library call with 80 calls, it's better to go through
> >>the string and replace them, maybe in console_write_ch or somewhere
> >>else.
> >That would be a one-liner.  Should I send such a patch?
> 
> Yes.

Replace the \0 character with a space to allow to use mvwaddchnstr for
full-screen updates in curses mode.


Signed-off-by: Bernhard Kauer <kauer@tudos.org>

diff --git a/console.h b/console.h
index 6def115..42ff822 100644
--- a/console.h
+++ b/console.h
@@ -306,6 +306,7 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
 typedef unsigned long console_ch_t;
 static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 {
+    if (!(ch & 0xff))  ch = 0x20;
     cpu_to_le32wu((uint32_t *) dest, ch);
 }

Re: [Qemu-devel] [PATCH] fix curses update - v2

[andrzej zaborowski]

Hi,

I pushed a modified patch to preserve attributes such as background
colour.  Please check if this works for you.

Cheers

Re: [Qemu-devel] [PATCH] fix curses update - v2

[Bernhard Kauer]

On Fri, May 21, 2010 at 02:02:43PM +0200, andrzej zaborowski wrote:
> I pushed a modified patch to preserve attributes such as background
> colour.

Good idea.

>  Please check if this works for you.

Yes, this works.


Thanks,

	Bernhard Kauer