Subject: Re: lib/36545: vi core dumps in a screen
To: None <blymn@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: Brett Lymn <blymn@baesystems.com.au>
List: netbsd-bugs
Date: 07/06/2007 14:30:02
The following reply was made to PR lib/36545; it has been noted by GNATS.

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/36545: vi core dumps in a screen
Date: Fri, 6 Jul 2007 23:55:11 +0930

 --NzB8fVQJ5HfG6fxh
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 
 OK - This all seems to stem from the fact that, for some unknown
 reason, the screen termcap has this for a key definition:
 
 kl=^@
 
 kl is the termcap entry for the left arrow key and we are being told
 that it will be the Null character (ascii 0).  Syntactically, this is
 a valid thing to do but since the kl termcap entry is supposed to be a
 string... well, that does not work well with C style strings.  We end
 up with a length of 0 which will cause us to negative index a
 character array a couple of times at the end of add_key_sequence().
 This would cause random clobbering of an entry in the key array for
 the keymap.  The negative indexing meant the bug was very sensitive to
 the stack layout - hence things "working" when the DEBUG version was
 built.  This bug has been around for a long time, I think the changes
 with wcurses forced it out of hiding.
 
 The attached patch resolves the problem for me...
 
 -- 
 Brett Lymn
 
 --NzB8fVQJ5HfG6fxh
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="pr36545_fix.diff"
 
 Index: getch.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libcurses/getch.c,v
 retrieving revision 1.48
 diff -u -r1.48 getch.c
 --- getch.c	28 May 2007 15:01:55 -0000	1.48
 +++ getch.c	6 Jul 2007 14:09:17 -0000
 @@ -396,6 +396,19 @@
  					 * base keymap. */
  	length = (int) strlen(sequence);
  
 +	/*
 +	 * OK - we really should never get a zero length string here, either
 +	 * the termcap entry is there and it has a value or we are not called
 +	 * at all.  Unfortunately, if someone assigns a termcap string to the
 +	 * ^@ value we get passed a null string which messes up our length.
 +	 * So, if we get a null string then just insert a leaf value in
 +	 * the 0th char position of the root keymap.  Note that we are
 +	 * totally screwed if someone terminates a multichar sequence
 +	 * with ^@... oh well.
 +	 */
 +	if (length == 0)
 +		length = 1;
 +
  	for (j = 0; j < length - 1; j++) {
  		  /* add the entry to the struct */
  		tmp_key = add_new_key(current, sequence[j], KEYMAP_MULTI, 0);
 
 --NzB8fVQJ5HfG6fxh--