NetBSD Problem Report #57072

From simonb@thistledown.com.au  Mon Oct 24 02:53:43 2022
Return-Path: <simonb@thistledown.com.au>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AA0351A921F
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 24 Oct 2022 02:53:43 +0000 (UTC)
Message-Id: <20221024025339.BB0511870@thoreau.thistledown.com.au>
Date: Mon, 24 Oct 2022 13:53:39 +1100 (AEDT)
From: Simon Burge <simonb@NetBSD.org>
Reply-To: Simon Burge <simonb@NetBSD.org>
To: gnats-bugs@NetBSD.org
Subject: nvi error "Error: move: l(24 + 0) c(80 + 0)"
X-Send-Pr-Version: 3.95

>Number:         57072
>Category:       bin
>Synopsis:       nvi error "Error: move: l(24 + 0) c(80 + 0)"
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    blymn
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Oct 24 02:55:01 +0000 2022
>Closed-Date:    Tue Nov 08 06:21:07 +0000 2022
>Last-Modified:  Tue Nov 08 06:21:07 +0000 2022
>Originator:     Simon Burge
>Release:        NetBSD 9.99.101
>Organization:
Not really.
>Environment:
	System: NetBSD thoreau.thistledown.com.au 9.99.101 NetBSD 9.99.101 (THOREAU.git) #58: Sun Oct 23 22:10:19 AEDT 2022 simonb@thoreau.thistledown.com.au:/NetBSD/netbsd-zfsboot-git/sys/arch/amd64/compile/THOREAU amd64
Architecture: x86_64
Machine: amd64
>Description:
	nvi gives the an error like
		Error: move: l(24 + 0) c(80 + 0)
	under certain conditions where it tries to write a long
	status line after an operation (eg, file write).
>How-To-Repeat:
	On a window that is 80 characters wide and:
	  jot 10 > /tmp/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
	  vi /tmp/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
	and type:
	  :w

>Fix:
	None given.

>Release-Note:

>Audit-Trail:
From: Rin Okuyama <rokuyama.rk@gmail.com>
To: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
 Simon Burge <simonb@netbsd.org>
Cc: Brett Lymn <blymn@NetBSD.org>
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Tue, 25 Oct 2022 21:52:59 +0900

 (blymn@ added to cc:)

 On 2022/10/24 11:55, Simon Burge wrote:
 >> Description:
 > 	nvi gives the an error like
 > 		Error: move: l(24 + 0) c(80 + 0)
 > 	under certain conditions where it tries to write a long
 > 	status line after an operation (eg, file write).
 >> How-To-Repeat:
 > 	On a window that is 80 characters wide and:
 > 	  jot 10 > /tmp/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
 > 	  vi /tmp/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
 > 	and type:
 > 	  :w

 I'm not sure whether nvi is not guilty here, but it seems that nvi hits
 a curses bug or its undefined behavior.

 If nvi is built with ncursesw from pkgsrc by this patch:

 https://gist.github.com/rokuyama/233bae41dc5cef273dfb73a64423a7fd

 the error does not occur.

 With extra CTRACE output by this patch:

 https://gist.github.com/rokuyama/1a7ea9f534805a302cc23c453cbf01fa

 the error turned out to occur as follows. For 80x24 xterm screen:

 (1) nvi outputs a 80-column message:

 /tmp/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: 10 lines, 21 characters.

 at the bottom line, i.e., (y, x) = (23, 0).

 (2) Cursor moves to (23, 80), and this (y, x) is returned to getsyx().

 (3) move(y, x) fails since x == win->maxx.

 Here is full CTRACE for nvi:

 http://www.netbsd.org/~rin/nvi-ctrace.20221025.gz

 Thanks,
 rin

From: Brett Lymn <blymn@internode.on.net>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
        Simon Burge <simonb@netbsd.org>, Brett Lymn <blymn@NetBSD.org>
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Wed, 26 Oct 2022 07:48:39 +1030

 On Tue, Oct 25, 2022 at 09:52:59PM +0900, Rin Okuyama wrote:
 > 
 > I'm not sure whether nvi is not guilty here, but it seems that nvi hits
 > a curses bug or its undefined behavior.
 > 

 I will have a look, writing to the bottom right corner can be...
 complicated.  It depends on a lot of things.  Thanks for the debug, I
 will sort through it and see what I can work out.

 -- 
 Brett Lymn
 --
 Sent from my NetBSD device.

 "We are were wolves",
 "You mean werewolves?",
 "No we were wolves, now we are something else entirely",
 "Oh"

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Brett Lymn <blymn@internode.on.net>
Cc: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
 Simon Burge <simonb@netbsd.org>, Brett Lymn <blymn@NetBSD.org>
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Wed, 26 Oct 2022 15:57:12 +0900

 On 2022/10/26 6:18, Brett Lymn wrote:
 > On Tue, Oct 25, 2022 at 09:52:59PM +0900, Rin Okuyama wrote:
 >>
 >> I'm not sure whether nvi is not guilty here, but it seems that nvi hits
 >> a curses bug or its undefined behavior.
 >>
 > 
 > I will have a look, writing to the bottom right corner can be...
 > complicated.  It depends on a lot of things.  Thanks for the debug, I
 > will sort through it and see what I can work out.

 Thank you for your help! And yes, it seems to depend precisely on
 situations. I tried to write a simplified code to reproduce the
 problem, but I couldn't...

 Thanks,
 rin

From: Simon Burge <simonb@NetBSD.org>
To: Rin Okuyama <rokuyama.rk@gmail.com>,
    Brett Lymn <blymn@internode.on.net>
Cc: gnats-bugs@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Wed, 26 Oct 2022 19:40:40 +1100

 Rin Okuyama wrote:

 > On 2022/10/26 6:18, Brett Lymn wrote:
 > > On Tue, Oct 25, 2022 at 09:52:59PM +0900, Rin Okuyama wrote:
 > >>
 > >> I'm not sure whether nvi is not guilty here, but it seems that nvi hits
 > >> a curses bug or its undefined behavior.
 > >>
 > > 
 > > I will have a look, writing to the bottom right corner can be...
 > > complicated.  It depends on a lot of things.  Thanks for the debug, I
 > > will sort through it and see what I can work out.
 >
 > Thank you for your help! And yes, it seems to depend precisely on
 > situations. I tried to write a simplified code to reproduce the
 > problem, but I couldn't...

 I haven't looked closely at the vi code, but would be easier/simplier
 or just plain more-hacky to just not try to put the cursor in the last
 column of the last row?  Already it deals with status messages longer
 than (window-width) chars by doing a multiline status message, so maybe
 just tweak that logic to kick in for (window-width - 1)?

 Or Brett can fix a complicated scenario :).

 Cheers,
 Simon.

From: RVP <rvp@SDF.ORG>
To: Simon Burge <simonb@NetBSD.org>
Cc: Rin Okuyama <rokuyama.rk@gmail.com>, Brett Lymn <blymn@internode.on.net>,
        gnats-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Wed, 26 Oct 2022 09:10:22 +0000 (UTC)

 On Wed, 26 Oct 2022, Simon Burge wrote:

 > I haven't looked closely at the vi code, but would be easier/simplier
 > or just plain more-hacky to just not try to put the cursor in the last
 > column of the last row?  Already it deals with status messages longer
 > than (window-width) chars by doing a multiline status message, so maybe
 > just tweak that logic to kick in for (window-width - 1)?
 >

 Here you go. Cooked this up a couple of days back, but, didn't send
 it in 'cuz like rin@ I noticed nvi behaved OK with ncurses, but didn't
 characterize the bug any further:

 ---START---
 diff -urN external/bsd/nvi.orig/dist/common/exf.c external/bsd/nvi/dist/common/exf.c
 --- external/bsd/nvi.orig/dist/common/exf.c	2014-01-26 21:43:45.000000000 +0000
 +++ external/bsd/nvi/dist/common/exf.c	2022-10-24 07:45:04.419673627 +0000
 @@ -996,6 +996,7 @@
   	 * a single screen column each, we can trim the filename.
   	 */
   	s = buf;
 +	len++;		/* account for '\n' added by msgq() */
   	if (len >= sp->cols) {
   		for (s = buf, t = buf + strlen(p); s < t &&
   		    (*s != '/' || len >= sp->cols - 3); ++s, --len);
 ---END---

 -RVP

From: Simon Burge <simonb@NetBSD.org>
To: RVP <rvp@SDF.ORG>
Cc: Rin Okuyama <rokuyama.rk@gmail.com>,
    Brett Lymn <blymn@internode.on.net>, gnats-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Thu, 27 Oct 2022 01:32:32 +1100

 RVP wrote:

 > On Wed, 26 Oct 2022, Simon Burge wrote:
 >
 > > I haven't looked closely at the vi code, but would be easier/simplier
 > > or just plain more-hacky to just not try to put the cursor in the last
 > > column of the last row?  Already it deals with status messages longer
 > > than (window-width) chars by doing a multiline status message, so maybe
 > > just tweak that logic to kick in for (window-width - 1)?
 > >
 >
 > Here you go. Cooked this up a couple of days back, but, didn't send
 > it in 'cuz like rin@ I noticed nvi behaved OK with ncurses, but didn't
 > characterize the bug any further:

 Thanks.  That got me thinking though that a more general fix^Whack would
 be to check in msgq() itself rather than one caller to msgq().  I'm not
 sure if this is called with any other longer strings, but this seems
 more robust.

 Still better for Brett to fix the bug if possible/easy/whatever!

 Cheers,
 Simon.

 Index: vs_msg.c
 ===================================================================
 RCS file: /cvsroot/src/external/bsd/nvi/dist/vi/vs_msg.c,v
 retrieving revision 1.6
 diff -d -p -u -r1.6 vs_msg.c
 --- vs_msg.c	26 Jan 2014 21:43:45 -0000	1.6
 +++ vs_msg.c	26 Oct 2022 09:51:52 -0000
 @@ -341,6 +341,11 @@ vs_msg(SCR *sp, mtype_t mtype, char *lin
  	 * XXX
  	 * There are almost certainly pathological cases that will break this
  	 * code.
 +	 *
 +	 * XXX
 +	 * Some versions of curses have problems writing to the last column
 +	 * in the last row.  Set maxcols to the number of columns-2 to work
 +	 * around this.
  	 */
  	if (IS_ONELINE(sp))
  		(void)msg_cmsg(sp, CMSG_CONT_S, &padding);
 @@ -348,7 +353,7 @@ vs_msg(SCR *sp, mtype_t mtype, char *lin
  		padding = 0;
  	padding += 2;

 -	maxcols = sp->cols - 1;
 +	maxcols = sp->cols - 2;
  	if (vip->lcontinue != 0) {
  		if (len + vip->lcontinue + padding > maxcols)
  			vs_output(sp, vip->mtype, ".\n", 2);

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Simon Burge <simonb@NetBSD.org>, RVP <rvp@SDF.ORG>
Cc: Brett Lymn <blymn@internode.on.net>, gnats-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Thu, 27 Oct 2022 11:14:20 +0900

 On 2022/10/26 23:32, Simon Burge wrote:
 > @@ -348,7 +353,7 @@ vs_msg(SCR *sp, mtype_t mtype, char *lin
 >   		padding = 0;
 >   	padding += 2;
 >   
 > -	maxcols = sp->cols - 1;
 > +	maxcols = sp->cols - 2;
 >   	if (vip->lcontinue != 0) {
 >   		if (len + vip->lcontinue + padding > maxcols)
 >   			vs_output(sp, vip->mtype, ".\n", 2);

 Yes, this works around the problem at the moment.

 Although, the real fix should be to
 - fix curses bug, or
 - stop nvi from relying on undefined behavior of curses.

 Thanks,
 rin

From: Brett Lymn <blymn@internode.on.net>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: Simon Burge <simonb@NetBSD.org>, RVP <rvp@SDF.ORG>, gnats-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Thu, 27 Oct 2022 16:44:25 +1030

 On Thu, Oct 27, 2022 at 11:14:20AM +0900, Rin Okuyama wrote:
 > 
 > Yes, this works around the problem at the moment.
 > 
 > Although, the real fix should be to
 > - fix curses bug, or

 well, I said this was complicated, see PR#56224 where the behaviour
 changed.  Admittedly the previous behaviour was wildly wrong but the
 logic to return ERR if a wrapping function cannot wrap makes sense sort
 of.  Unfortunately susV2 is not really clear on how this should be
 handled.  It is interesting that this works with ncurses, if you follow
 the email trail there was a claim made that ncurses returned ERR in this
 situation too - perhaps that has changed?

 > - stop nvi from relying on undefined behavior of curses.
 > 

 I don't know if it is deliberate or not but it seems like nvi almost
 gets this right, it does a addstr of "." in the last column - if this
 addstr was an insch then things would be ok.  I used this in the slk
 support to write to the bottom right character, the last character is
 done as a insch which does not advance the cursor so will not cause a
 scroll and hence avoids the whole problem.

 -- 
 Brett Lymn
 --
 Sent from my NetBSD device.

 "We are were wolves",
 "You mean werewolves?",
 "No we were wolves, now we are something else entirely",
 "Oh"

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Thu, 27 Oct 2022 13:03:09 +0300

 On Thu, Oct 27, 2022 at 06:15:02 +0000, Brett Lymn wrote:

 >  well, I said this was complicated, see PR#56224 where the behaviour
 >  changed.

 Oh, I seems to have missed this one, or I would have meekly protested :)
 Cf. PR#30978 for previous iteration of this problem.


 >  gets this right, it does a addstr of "." in the last column - if this
 >  addstr was an insch then things would be ok.  I used this in the slk

 I thought curses is supposed to handle this internally based on TERM
 capabilities.  If the termial requires insert char hack to write into
 the bottom-right corner, addstr &c should perform it behind the
 scenes.


 -uwe

From: "Brett Lymn" <blymn@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57072 CVS commit: src/lib/libcurses
Date: Fri, 4 Nov 2022 06:12:23 +0000

 Module Name:	src
 Committed By:	blymn
 Date:		Fri Nov  4 06:12:23 UTC 2022

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

 Log Message:
 PR bin/57072

 This fixes observed behviour in the PR.  Allow the cursor to be
 moved one past the EOL, if postitioned here then set ISPASTEOL.
 also protect out of range access if win->cury is past maxy.


 To generate a diff of this commit:
 cvs rdiff -u -r1.24 -r1.25 src/lib/libcurses/move.c

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

From: Brett Lymn <blymn@internode.on.net>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: Simon Burge <simonb@NetBSD.org>, RVP <rvp@SDF.ORG>, gnats-bugs@netbsd.org
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Fri, 4 Nov 2022 16:49:54 +1030

 On Thu, Oct 27, 2022 at 11:14:20AM +0900, Rin Okuyama wrote:
 > 
 > Although, the real fix should be to
 > - fix curses bug, or

 I have committed a fix for this.  The issue does not occur for me now.

 -- 
 Brett Lymn
 --
 Sent from my NetBSD device.

 "We are were wolves",
 "You mean werewolves?",
 "No we were wolves, now we are something else entirely",
 "Oh"

From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/57072: nvi error "Error: move: l(24 + 0) c(80 + 0)"
Date: Sun, 6 Nov 2022 09:37:21 +0000 (UTC)

 On Fri, 4 Nov 2022, Brett Lymn wrote:

 > On Thu, Oct 27, 2022 at 11:14:20AM +0900, Rin Okuyama wrote:
 > >
 > > Although, the real fix should be to
 > > - fix curses bug, or
 >
 > I have committed a fix for this.  The issue does not occur for me now.
 >

 Curses now adds an extra blank line when printing lines > COLS and it has
 to scroll. Run the program on the source itself in an 80x25 window:

 ---START---
 #ifdef USE_NCURSES
 #include <ncursesw/ncurses.h>
 #else
 #include <curses.h>
 #endif

 int
 main(int argc, char* argv[])
 {
  	char line[BUFSIZ];
  	WINDOW* w;
  	FILE* fp;

  	if (argc != 2 || (fp = fopen(argv[1], "r")) == NULL)
  		return 1;

  	w = initscr();
  	scrollok(w, TRUE);
  	while (fgets(line, sizeof line, fp) != NULL) {
  		waddstr(w, line);
  		wrefresh(w);
  	}
  	waddstr(w, "Enter to QUIT: ");
  	getch();
  	endwin();
  	fclose(fp);
  	return 0;
 }

 #if 0
 uuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu
 vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
 #endif
 ---END---

 -RVP

Responsible-Changed-From-To: bin-bug-people->blymn
Responsible-Changed-By: blymn@NetBSD.org
Responsible-Changed-When: Tue, 08 Nov 2022 06:21:07 +0000
Responsible-Changed-Why:
I claim this one.


State-Changed-From-To: open->closed
State-Changed-By: blymn@NetBSD.org
State-Changed-When: Tue, 08 Nov 2022 06:21:07 +0000
State-Changed-Why:
Fixed a bug in libcurses wmove function, vi no longer misbehaves.


>Unformatted:
 	NetBSD-current as of 21 Oct 2022, but has been an issue
 	for many years.

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2022 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.