Date: Thu, 6 Sep 2012 19:10:07 GMT From: Mark Johnston <markjdb@gmail.com> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/169773: sh(1): Resizing causes /bin/sh to repeat edit operations Message-ID: <201209061910.q86JA7Ve099671@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/169773; it has been noted by GNATS. From: Mark Johnston <markjdb@gmail.com> 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 <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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201209061910.q86JA7Ve099671>