NetBSD Problem Report #41257

From jld@xlerb.net  Tue Apr 21 06:06:14 2009
Return-Path: <jld@xlerb.net>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id BCF6663B8A5
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 21 Apr 2009 06:06:14 +0000 (UTC)
Message-Id: <20090421060609.7245B15088@planetarium.xlerb.net>
Date: Tue, 21 Apr 2009 02:06:09 -0400 (EDT)
From: jld@panix.com
Reply-To: jld@panix.com
To: gnats-bugs@gnats.NetBSD.org
Subject: curses: getyx + wmove violates least astonishment past end-of-line
X-Send-Pr-Version: 3.95

>Number:         41257
>Category:       lib
>Synopsis:       curses: getyx + wmove violates least astonishment past end-of-line
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    blymn
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 21 06:10:00 +0000 2009
>Closed-Date:    Thu Sep 27 10:15:21 +0000 2018
>Last-Modified:  Thu Sep 27 10:15:21 +0000 2018
>Originator:     Jed Davis
>Release:        NetBSD 5.0_RC2
>Organization:
>Environment:
System: NetBSD planetarium.xlerb.net 5.0_RC2 NetBSD 5.0_RC2 (PLANETAR64) #3: Tue Mar 3 21:40:33 EST 2009 jld@city-of-dreadful-night.xlerb.net:/usr/local/src/nb-5/sys/arch/amd64/compile/PLANETAR64 amd64
Architecture: x86_64
Machine: amd64
>Description:

If a curses library call that advances the cursor, like waddch, advances
the cursor past the end of the line, then getyx() sets the y argument
to that line and the x argument to the column one past the right edge
of the window.  If these values are then passed back to wmove, it will
return ERR because the x coordinate is out of range.

>How-To-Repeat:

#include <curses.h>
#include <stdlib.h>
#include <string.h>

int main()
{
	WINDOW *w;
	int maxx, y, x, rc;
	char *s;

	w = initscr();
	maxx = getmaxx(w);
	s = malloc(maxx + 1);
	memset(s, 'x', maxx);
	s[maxx] = '\0';
	mvaddstr(0, 0, s);
	getyx(w, y, x);
	addch('#');
	rc = move(y, x);
	sprintf(s, "wmove(%d, %d) = %s\n", y, x, (rc == OK ? "OK" : "ERR"));
	mvaddstr(2, 0, s);
	getch();
	return OK != endwin();
}

/* Compare the behavior with that of other curses implementations. */

>Fix:

The SUSv2 says that "Some /add/ functions move the cursor just beyond
the end of the last character added.  If this position is beyond the end
of a line, it causes wrapping and scrolling ..."[*], which suggests to
me that the wrapping of the cursor position is meant to occur during the
operation that moves the cursor off the end of the line.

[*] http://opengroup.org/onlinepubs/007908775/xcurses/intov.html#tag_001_004_002_002

I have prepared a patch that modifies getcurx() and getcury() to report
the cursor as being at the beginning of the next line, if the past-EOL
flag is set, at http://www.NetBSD.org/~jld/nbcurses-getyx-pasteol.diff
and included below:

Index: lib/libcurses/getyx.c
===================================================================
RCS file: /bag/nb/repo/src/lib/libcurses/getyx.c,v
retrieving revision 1.5
diff -u -p -r1.5 getyx.c
--- lib/libcurses/getyx.c	28 Apr 2008 20:23:01 -0000	1.5
+++ lib/libcurses/getyx.c	21 Apr 2009 04:12:36 -0000
@@ -80,6 +80,8 @@ getparx(WINDOW *win)
 int
 getcury(WINDOW *win)
 {
+	if (win->lines[win->cury]->flags & __ISPASTEOL)
+		return win->cury + 1;
 	return(win->cury);
 }

@@ -90,6 +92,8 @@ getcury(WINDOW *win)
 int
 getcurx(WINDOW *win)
 {
+	if (win->lines[win->cury]->flags & __ISPASTEOL)
+		return 0;
 	return(win->curx);
 }

This may not be the Right Thing, but in simple testing it does appear
to work, and fix the original application that was having trouble.
Alternately, wmove() could be modified to accept one-past-the-end
coordinates as input and set up the state appropriately, but I'm not
familiar enough with the curses internals to know if that would have
unintended consequences.

>Release-Note:

>Audit-Trail:
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment past end-of-line
Date: Tue, 21 Apr 2009 08:07:23 +0100

 On Tue, Apr 21, 2009 at 06:10:01AM +0000, jld@panix.com wrote:
 > >Number:         41257
 > >Category:       lib
 > >Synopsis:       curses: getyx + wmove violates least astonishment past end-of-line
 > 
 > I have prepared a patch that modifies getcurx() and getcury() to report
 > the cursor as being at the beginning of the next line, if the past-EOL
 > flag is set, at http://www.NetBSD.org/~jld/nbcurses-getyx-pasteol.diff
 > and included below:

 What happens in the bottom RH corner ?

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Brett Lymn <blymn@baesystems.com.au>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment past end-of-line
Date: Tue, 21 Apr 2009 23:08:37 +0930

 Try this patch, it makes the test program behave as expected for me:

 Index: addbytes.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libcurses/addbytes.c,v
 retrieving revision 1.34
 diff -u -r1.34 addbytes.c
 --- addbytes.c	4 Jul 2008 16:52:10 -0000	1.34
 +++ addbytes.c	21 Apr 2009 13:36:36 -0000
 @@ -540,7 +540,7 @@
  		/* Mark as "continuation" cell */
  		tp->attr |= __WCWIDTH;
  	}
 -	if (*x == win->maxx) {
 +	if (*x >= win->maxx - 1) {
  		(*lnp)->flags |= __ISPASTEOL;
  		newx = win->maxx - 1 + win->ch_off;
  		if (newx > *(*lnp)->lastchp)

 -- 
 Brett Lymn
 "Warning:
 The information contained in this email and any attached files is
 confidential to BAE Systems Australia. If you are not the intended
 recipient, any use, disclosure or copying of this email or any
 attachments is expressly prohibited.  If you have received this email
 in error, please notify us immediately. VIRUS: Every care has been
 taken to ensure this email and its attachments are virus free,
 however, any loss or damage incurred in using this email is not the
 sender's responsibility.  It is your responsibility to ensure virus
 checks are completed before installing any data sent in this email to
 your computer."


From: Brett Lymn <blymn@baesystems.com.au>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment past end-of-line
Date: Tue, 21 Apr 2009 22:53:20 +0930

 On Tue, Apr 21, 2009 at 06:10:01AM +0000, jld@panix.com wrote:
 > 
 > This may not be the Right Thing, but in simple testing it does appear
 > to work, and fix the original application that was having trouble.
 > Alternately, wmove() could be modified to accept one-past-the-end
 > coordinates as input and set up the state appropriately, but I'm not
 > familiar enough with the curses internals to know if that would have
 > unintended consequences.

 No, it is not the correct thing to do - the x & y coordinates should
 always be correct after adding bytes trying to paper over the cracks
 by diddling the reporting routines will result in pain, lossage and
 confusion because the internal cursor state is still wrong.

 All the *add* functions end up calling a couple of routines in
 addbytes.c, _cursesi_addwchar() and _cursesi_addbyte().  I suspect the
 problem arises where the current x position is compared for equality
 with (maxx - 1), in the narrow character case this would probably work
 but in the wide character case we may skip ahead a few characters when
 inserting a wide character so the equality check would fail, this
 probably needs to be >= instead of ==

 -- 
 Brett Lymn
 "Warning:
 The information contained in this email and any attached files is
 confidential to BAE Systems Australia. If you are not the intended
 recipient, any use, disclosure or copying of this email or any
 attachments is expressly prohibited.  If you have received this email
 in error, please notify us immediately. VIRUS: Every care has been
 taken to ensure this email and its attachments are virus free,
 however, any loss or damage incurred in using this email is not the
 sender's responsibility.  It is your responsibility to ensure virus
 checks are completed before installing any data sent in this email to
 your computer."


Responsible-Changed-From-To: lib-bug-people->blymn
Responsible-Changed-By: blymn@NetBSD.org
Responsible-Changed-When: Wed, 22 Apr 2009 13:57:41 +0000
Responsible-Changed-Why:
I have already provided a patch for this I may as well have the PR


State-Changed-From-To: open->feedback
State-Changed-By: blymn@NetBSD.org
State-Changed-When: Wed, 22 Apr 2009 13:57:41 +0000
State-Changed-Why:
Awaiting feedback from reporter regarding the patch which corrected
the bug demonstrated by the test code.


From: Jed Davis <jld@panix.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment
	past end-of-line
Date: Thu, 7 May 2009 20:55:56 -0400

 On Tue, Apr 21, 2009 at 01:45:03PM +0000, Brett Lymn wrote:
 >  --- addbytes.c	4 Jul 2008 16:52:10 -0000	1.34
 >  +++ addbytes.c	21 Apr 2009 13:36:36 -0000
 >  @@ -540,7 +540,7 @@
 >   		/* Mark as "continuation" cell */
 >   		tp->attr |= __WCWIDTH;
 >   	}
 >  -	if (*x == win->maxx) {
 >  +	if (*x >= win->maxx - 1) {
 >   		(*lnp)->flags |= __ISPASTEOL;
 >   		newx = win->maxx - 1 + win->ch_off;
 >   		if (newx > *(*lnp)->lastchp)

 I've finally gotten a chance to test this, and it seems to be not quite
 right -- it causes the wrapping to occur one character too soon, so the
 last column of the terminal can't be written to.

 --Jed

From: Jed Davis <jld@panix.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment
	past end-of-line
Date: Thu, 7 May 2009 21:14:21 -0400

 On Tue, Apr 21, 2009 at 02:00:06PM +0000, Brett Lymn wrote:
 >  No, it is not the correct thing to do - the x & y coordinates should
 >  always be correct after adding bytes trying to paper over the cracks
 >  by diddling the reporting routines will result in pain, lossage and
 >  confusion because the internal cursor state is still wrong.

 But assuming that the one-past-the-end internal state is a reasonable
 state to be in -- which the presence of the __ISPASTEOL machinery seems
 to suggest -- then the alternative to hiding that state from the user is
 to allow it to be retrieved and restored, thusly:

 Index: move.c
 ===================================================================
 RCS file: /bag/nb/repo/src/lib/libcurses/move.c,v
 retrieving revision 1.15
 diff -u -p -r1.15 move.c
 --- move.c	21 Jan 2007 13:25:36 -0000	1.15
 +++ move.c	8 May 2009 01:07:31 -0000
 @@ -67,11 +67,14 @@ wmove(WINDOW *win, int y, int x)
  #endif
  	if (x < 0 || y < 0)
  		return (ERR);
 -	if (x >= win->maxx || y >= win->maxy)
 +	if (x > win->maxx || y >= win->maxy)
  		return (ERR);
  	win->curx = x;
  	win->lines[win->cury]->flags &= ~__ISPASTEOL;
  	win->cury = y;
 -	win->lines[y]->flags &= ~__ISPASTEOL;
 +	if (x == win->maxx)
 +		win->lines[y]->flags |= __ISPASTEOL;
 +	else
 +		win->lines[y]->flags &= ~__ISPASTEOL;
  	return (OK);
  }

 I do agree that the == in addbytes.c should be a >=, regardless.

 --Jed

From: Jed Davis <jld@panix.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment
	past end-of-line
Date: Fri, 8 May 2009 20:24:32 -0400

 On Fri, May 08, 2009 at 01:15:03AM +0000, Jed Davis wrote:
 >  
 >  But assuming that the one-past-the-end internal state is a reasonable
 >  state to be in -- which the presence of the __ISPASTEOL machinery seems
 >  to suggest -- then the alternative to hiding that state from the user is
 >  to allow it to be retrieved and restored, thusly:

 Except that something is wrong here.  If I write a character which
 extends up to (but not beyond) the rightmost column of the terminal
 with addch or add_wch, then it sets __ISPASTEOL and leaves the cursor
 position at the *beginning* of the character.  It's only if I use addstr
 that the cursor gets into the one-past-the-end state.

 --Jed

From: Brett Lymn <blymn@baesystems.com.au>
To: gnats-bugs@netbsd.org
Cc: blymn@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
        jld@panix.com
Subject: Re: lib/41257: curses: getyx + wmove violates least astonishment past end-of-line
Date: Sat, 9 May 2009 22:08:14 +0930

 On Sat, May 09, 2009 at 12:25:02AM +0000, Jed Davis wrote:
 >  
 >  Except that something is wrong here.  If I write a character which
 >  extends up to (but not beyond) the rightmost column of the terminal
 >  with addch or add_wch, then it sets __ISPASTEOL and leaves the cursor
 >  position at the *beginning* of the character.  It's only if I use addstr
 >  that the cursor gets into the one-past-the-end state.
 >  

 Right.  That is why I said previously that trying to paper over the
 bug by adjusting the return of getyx() is not a good idea.  I will
 generate another patch soon that should remove the off by one error.

 -- 
 Brett Lymn
 "Warning:
 The information contained in this email and any attached files is
 confidential to BAE Systems Australia. If you are not the intended
 recipient, any use, disclosure or copying of this email or any
 attachments is expressly prohibited.  If you have received this email
 in error, please notify us immediately. VIRUS: Every care has been
 taken to ensure this email and its attachments are virus free,
 however, any loss or damage incurred in using this email is not the
 sender's responsibility.  It is your responsibility to ensure virus
 checks are completed before installing any data sent in this email to
 your computer."


State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 03 May 2010 03:27:53 +0000
State-Changed-Why:
Feedback on this one was also received a year ago.


State-Changed-From-To: open->closed
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Thu, 27 Sep 2018 12:15:21 +0200
State-Changed-Why:
Works in HEAD (NetBSD 8.99.25 amd64)
No ERR is returned, NetBSD curses(3) wraps the result accordingly.
5.x is EOL so close this report.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.