From owner-freebsd-bugs@FreeBSD.ORG Thu Sep 6 19:10:08 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 54A14106566B for ; Thu, 6 Sep 2012 19:10:08 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 25C5B8FC16 for ; Thu, 6 Sep 2012 19:10:08 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q86JA7OV099690 for ; Thu, 6 Sep 2012 19:10:07 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q86JA7Ve099671; Thu, 6 Sep 2012 19:10:07 GMT (envelope-from gnats) Date: Thu, 6 Sep 2012 19:10:07 GMT Message-Id: <201209061910.q86JA7Ve099671@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Mark Johnston Cc: Subject: Re: bin/169773: sh(1): Resizing causes /bin/sh to repeat edit operations X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Mark Johnston List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Sep 2012 19:10:08 -0000 The following reply was made to PR bin/169773; it has been noted by GNATS. From: Mark Johnston To: bug-followup@FreeBSD.org, peter@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 #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--