bin/169773: sh(1): Resizing causes /bin/sh to repeat edit operations

[Mark Johnston]

The following reply was made to PR bin/169773; it has been noted by GNATS.

From: Mark Johnston <markjdb at gmail.com>
To: bug-followup at FreeBSD.org, peter at rulingia.com
Cc:  
Subject: Re: bin/169773: sh(1): Resizing causes /bin/sh to repeat edit
 operations
Date: Thu, 6 Sep 2012 15:03:36 -0400

 --HlL+5n6rz5pIUxbD
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 The root of the problem is some questionable error-handling in libedit.
 
 First, read_char() doesn't do the right thing if its call to
 read(2) is interrupted by a signal (SIGWINCH in this case). Basically,
 it retries the read once, and returns an error if the second read failed
 (which is why it takes two SIGWINCHs to trigger the delete). IMHO,
 read_char() should always retry the read(2) if its interrupted by a
 signal. The attached patched fixes this.
 
 The second problem has to do with how read(2) errors are handled higher
 up in libedit's stack. We have read_getcmd(), which calls el_getc(),
 which calls the read_char() function mentioned above. read_char()
 returns -1 on error, and then -1 gets returned to read_getcmd() by
 el_getc(). el_getc() returns OKCMD on success, and OKCMD is defined to
 be... -1. Thus read_getcmd() has no idea that an error occured and ends
 up reusing a local variable (cmdnum) which causes the second backspace.
 
 I think the fix here is to define OKCMD to be 1 (the value returned by
 read_char() on success). For the purpose of fixing the bug reported
 here, the check for EINTR is enough, but this second bug should probably
 be fixed too. I'm happy to test alternative fixes. =)
 
 Thanks,
 -Mark
 
 --HlL+5n6rz5pIUxbD
 Content-Type: text/x-diff; charset=us-ascii
 Content-Disposition: attachment; filename="libedit_error_handling.patch"
 
 diff --git a/lib/libedit/read.c b/lib/libedit/read.c
 index 7d7f54b..013f93e 100644
 --- a/lib/libedit/read.c
 +++ b/lib/libedit/read.c
 @@ -49,7 +49,7 @@ __FBSDID("$FreeBSD$");
  #include <stdlib.h>
  #include "el.h"
  
 -#define	OKCMD	-1
 +#define	OKCMD	1
  
  private int	read__fixio(int, int);
  private int	read_preread(EditLine *);
 @@ -169,9 +169,6 @@ read__fixio(int fd __unused, int e)
  #endif /* TRY_AGAIN */
  		return (e ? 0 : -1);
  
 -	case EINTR:
 -		return (0);
 -
  	default:
  		return (-1);
  	}
 @@ -295,9 +292,11 @@ read_char(EditLine *el, char *cp)
   again:
  	el->el_signal->sig_no = 0;
  	while ((num_read = read(el->el_infd, cp, 1)) == -1) {
 -		if (el->el_signal->sig_no == SIGCONT) {
 -			sig_set(el);
 -			el_set(el, EL_REFRESH);
 +		if (errno == EINTR) {
 +			if (el->el_signal->sig_no == SIGCONT) {
 +				sig_set(el);
 +				el_set(el, EL_REFRESH);
 +			}
  			goto again;
  		}
  		if (!tried && read__fixio(el->el_infd, errno) == 0)
 
 --HlL+5n6rz5pIUxbD--