The following reply was made to PR bin/169773; it has been noted by GNATS.
From: Mark Johnston <mark...@gmail.com>
To: bug-follo...@freebsd.org, pe...@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--
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
