NetBSD Problem Report #45791

From www@NetBSD.org  Fri Jan  6 15:02:57 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id EC27763B84C
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  6 Jan 2012 15:02:56 +0000 (UTC)
Message-Id: <20120106150256.0095963B11E@www.NetBSD.org>
Date: Fri,  6 Jan 2012 15:02:55 +0000 (UTC)
From: nathanialsloss@yahoo.com.au
Reply-To: nathanialsloss@yahoo.com.au
To: gnats-bugs@NetBSD.org
Subject: libcurses - getnstr erase character weirdness
X-Send-Pr-Version: www-1.0

>Number:         45791
>Category:       lib
>Synopsis:       libcurses - getnstr erase character weirdness
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    christos
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jan 06 15:05:00 +0000 2012
>Closed-Date:    Tue Jan 31 08:30:48 +0000 2012
>Last-Modified:  Tue Jan 31 08:30:48 +0000 2012
>Originator:     Nat Sloss
>Release:        NetBSD 5.0.1
>Organization:
>Environment:
NetBSD beast 5.0.1 NetBSD 5.0.1 (BEAST) #94: Thu Jan  5 04:19:56 EST 2012  build@beast:/home/build/NetBSD-5.0.1_source_tree/usr/src/sys/arch/i386/compile/BEAST i386

>Description:
Functions getnstr and similar display the erase character backspace incorrectly from a text terminal.

On NetBSD 5.0.1 backspace moves forward, on current 5.99.58 it displays a ^ not a circumflex but an up arrow kind of symbol.

Tabs also cannot be deleted.
>How-To-Repeat:
Write a function that uses getnstr for instance:

#include <stdio.h>
#include <curses.h>

int main() {
    char buff[10]
    initscr();
    mvprintw( 24, 0, "Sample prompt:" );
    fflush( stdout );
    refresh();

    fflush( stdin );
    getnstr( buff, 5 );

    endwin();

    return 0;
}

Compile the example above.

Input a character and hit backspace and you will see the weird behavior.  Also entering a character then pressing tab and then hit backspace.  You will notice that you cant delete the character the backspace only goes back so far.

Then try "Control-H" repeatedly from a text terminal and you will be able to delete the prompt.






>Fix:
I patched get str.c adding a new variables xpos and asciimin.

xpos contains the current x position for inputted characters.

asciimin is the value of ascii 'space'.

This patch also displays tabs and less than asciimin characters as single spaces to make it easier to delete them.  It fixes the backspace and "Control-H" anomalies for wscons, konsole and xterm.

--- getstr.c.orig       2007-05-29 01:01:55.000000000 +1000
+++ getstr.c    2012-01-03 04:17:46.000000000 +1100
@@ -160,13 +160,14 @@
 int
 __wgetnstr(WINDOW *win, char *str, int n)
 {
-       char *ostr, ec, kc;
-       int c, oldx, remain;
+       char *ostr, ec, kc, asciimin;
+       int c, oldx, xpos, remain;

        ostr = str;
        ec = erasechar();
        kc = killchar();
-       oldx = win->curx;
+       asciimin = 0x20;
+       oldx = xpos = win->curx;
        _DIAGASSERT(n == -1 || n > 1);
        remain = n - 1;

@@ -182,17 +183,25 @@
                        *str = '\0';
                        if (str != ostr) {
                                if ((char) c == ec) {
-                                       mvwaddch(win, win->cury, win->curx,
+                                       mvwaddch(win, win->cury, xpos,
                                            ' ');
-                                       wmove(win, win->cury, win->curx - 1);
+                                       if (xpos > oldx)
+                                           mvwaddch(win, win->cury, xpos - 1,
+                                               ' ');
+                                       if (win->curx > xpos - 1)
+                                           wmove(win, win->cury, xpos - 1);
+
+                                       xpos--;
                                }
                                if (c == KEY_BACKSPACE || c == KEY_LEFT) {
                                        /* getch() displays the key sequence */
-                                       mvwaddch(win, win->cury, win->curx - 1,
-                                           ' ');
-                                       mvwaddch(win, win->cury, win->curx - 2,
+                                       mvwaddch(win, win->cury, win->curx,
                                            ' ');
-                                       wmove(win, win->cury, win->curx - 1);
+                                       mvwaddch(win, win->cury, win->curx - 1,
+                                           ' ');
+                                       if (win->curx > xpos)
+                                           wmove(win, win->cury, xpos - 1);
+                                       xpos--;
                                }
                                str--;
                                if (n != -1) {
@@ -200,11 +209,13 @@
                                        remain++;
                                }
                        } else {        /* str == ostr */
-                               if (c == KEY_BACKSPACE || c == KEY_LEFT)
+                               if (c == ec || c == KEY_BACKSPACE || c == KEY_LEFT)
                                        /* getch() displays the other keys */
-                                       mvwaddch(win, win->cury, win->curx - 1,
-                                           ' ');
+                                       if (win->curx > oldx)
+                                           mvwaddch(win, win->cury,
+                                               win->curx - 1, ' ');
                                wmove(win, win->cury, oldx);
+                               xpos = oldx;
                        }
                } else if (c == kc) {
                        *str = '\0';
@@ -229,15 +240,21 @@
                        wmove(win, win->cury, oldx);
                } else if (c >= KEY_MIN && c <= KEY_MAX) {
                        /* getch() displays these characters */
-                       mvwaddch(win, win->cury, win->curx - 1, ' ');
-                       wmove(win, win->cury, win->curx - 1);
+                       mvwaddch(win, win->cury, xpos, ' ');
+                       wmove(win, win->cury, xpos);
                } else {
+                       if ((c < asciimin) && remain) {
+                               mvwaddch(win, win->cury, xpos, ' ');
+                               wmove(win, win->cury, xpos + 1);
+                       }
+
                        if (remain) {
                                str++;
+                               xpos++;
                                remain--;
                        } else {
-                               mvwaddch(win, win->cury, win->curx - 1, ' ');
-                               wmove(win, win->cury, win->curx - 1);
+                               mvwaddch(win, win->cury, xpos, ' ');
+                               wmove(win, win->cury, xpos);
                        }
                }
        }


The only key it has trouble with is the escape key it moves forward a single space as I made it do but it erases the previous character I think it's a getch issue and did not want to change it.

I hope my patch is appropriate and that it helps.

Regards,

Nat.

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45791 CVS commit: src/lib/libcurses
Date: Fri, 6 Jan 2012 17:20:54 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Fri Jan  6 22:20:54 UTC 2012

 Modified Files:
 	src/lib/libcurses: getstr.c

 Log Message:
 PR/45791: Nat Sloss: getnstr erase character weirdness
 Fix processing of backspace erase char and char left.


 To generate a diff of this commit:
 cvs rdiff -u -r1.20 -r1.21 src/lib/libcurses/getstr.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: lib-bug-people->christos
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Sat, 07 Jan 2012 00:17:44 +0000
Responsible-Changed-Why:
christos committed something, ok to close?


State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sat, 07 Jan 2012 00:17:44 +0000
State-Changed-Why:


From: Nat Sloss <nathanialsloss@yahoo.com.au>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/45791
Date: Fri, 27 Jan 2012 15:19:45 +1100

 Hi,

 Thanks Christos for the quick commit.  I tried it out and it works well.

 Although it introduces a new problem as it differs from my patch when it comes 
 to cursor placement when no more characters remaining.  Consider this:

 getnstr( somebuff, 5);  -  As in the previous example.

 Now at the prompt enter in 4 characters. You'll notice the cursor is in the 5 
 character position.  Enter in another character.  This character is not 
 echoed and the cursor moves back to the fourth position.  Enter in yet 
 another character and this character is echoed as the fourth character and 
 the cursor moves back to the fourth character position. Every character after 
 the initial four characters is not considered as input although echoed.

 I think this was a simple mistake as the changes display a space at xpos when 
 remaining == 0, but then moves the cursor xpos - 1 instead of xpos.  

 As I think the cursor should stay at the fifth character position for any 
 characters received after the initial four characters.

 I suggest one of the following fixes:

 Patch 1: move xpos when remaining - 0;

 --- getstr.c.orig	2012-01-07 17:53:19.000000000 +1100
 +++ getstr.c	2012-01-26 13:58:13.000000000 +1100
 @@ -250,7 +250,7 @@
  				remain--;
  			} else {
  				mvwaddch(win, win->cury, xpos, ' ');
 -				wmove(win, win->cury, xpos - 1);
 +				wmove(win, win->cury, xpos);
  			}
  		}
  	}



 or patch 2:  It saves a line and moves xpos for all characters.

 --- getstr.c.orig	2012-01-07 17:53:19.000000000 +1100
 +++ getstr.c	2012-01-26 14:00:50.000000000 +1100
 @@ -243,15 +243,14 @@
  			if (remain) {
  				if (iscntrl((unsigned char)c)) {
  					mvwaddch(win, win->cury, xpos, ' ');
 -					wmove(win, win->cury, xpos + 1);
  				}
  				str++;
  				xpos++;
  				remain--;
  			} else {
  				mvwaddch(win, win->cury, xpos, ' ');
 -				wmove(win, win->cury, xpos - 1);
  			}
 +			wmove(win, win->cury, xpos);
  		}
  	}

 I don't know which is better and I don't think these routines are UTF-8/16 
 ready so probably patch two in my opinion as it is a little more succinct.


 Anyhow once again thanks for the quick commit, sorry for the slow reply I'm 
 still in holiday mode.

 Regards,

 Nat

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	nathanialsloss@yahoo.com.au
Cc: 
Subject: Re: lib/45791
Date: Fri, 27 Jan 2012 10:38:03 -0500

 On Jan 27,  4:30am, nathanialsloss@yahoo.com.au (Nat Sloss) wrote:
 -- Subject: Re: lib/45791

 |  I don't know which is better and I don't think these routines are UTF-8/16 
 |  ready so probably patch two in my opinion as it is a little more succinct.

 Yes, I followed your advice :-)

 |  Anyhow once again thanks for the quick commit, sorry for the slow reply I'm 
 |  still in holiday mode.

 No problem, I am glad someone is still hin holiday mode! Make it last :-)

 christos

From: Nat Sloss <nathanialsloss@yahoo.com.au>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/45791
Date: Tue, 31 Jan 2012 13:41:33 +1100

 Hi.

 I've tested the latest changes in both 5.0.1 and current and they work great.

 So it's ok to close this pr.

 Many thanks to Christos and all who worked on this bug.

 Regards,

 Nat.

State-Changed-From-To: feedback->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Tue, 31 Jan 2012 08:30:48 +0000
State-Changed-Why:
Confirmed fixed, thanks.


>Unformatted:

Home	PR Database Search