Hi,
deraadt@ recently reported to me that the editline(3) library, while
line editing is active - for example inside el_gets(3) - ignores
the first SIGINT it receives, for example the the first Ctrl-C the
user presses. I consider that a bug in the editline(3) library.
Some programs, for example our old ftp(1) implementation, work
around the bug in a horrible way by using setjmp(3)/longjmp(3).
The root cause of the problem is in libedit/read.c, in the interaction
of the functions read_char() and read__fixio(). Before fixing the bug
can reasonably be considered, the function read__fixio() direly needs
cleanup. As it stands, it is utterly unreadable.
So here is a patch to make it clear what the function does, with
no functional change intended yet (-37 +5 LOC).
There will be one or more follow-up patches. If you want to receive
them, please reply to me, i won't necessarily send them all to
tech@.
I see some value in avoiding gratuitious divergence from NetBSD,
but getting rid of this horrible mess is not gratuitious.
Rationale:
* Do not mark an argument as unused that is actually used.
* errno cannot possibly be -1. Even is it were, treating it as
EAGAIN makes no sense, treating it as the most severe error
imaginable makes more sense to me.
* We define EWOULDBLOCK to be the same as EAGAIN, so no need
to handle it separately.
* No need to #define TRY_AGAIN to use it just once.
* Don't do the same thing twice. We do support the FIONBIO ioctl(2),
so the the indirection using the F_GETFL fcntl(2) can be deleted.
* No need to play confusing games with the "e" variable.
Just return -1 for error or 0 for success in a straightforward
manner.
OK?
Ingo
P.S.
I also considered whether this FIONBIO dance should better be done
at the initialization stage rather than after EAGAIN already happened.
But maybe not. This is a library. The application program might
set the fd to non-blocking mode at any time and then call el_gets(3)
again, in which case the library needs to restore blocking I/O to
do its work.
Index: read.c
===================================================================
RCS file: /cvs/src/lib/libedit/read.c,v
retrieving revision 1.44
diff -u -p -r1.44 read.c
--- read.c 25 May 2016 09:36:21 -0000 1.44
+++ read.c 8 Aug 2021 10:30:06 -0000
@@ -39,9 +39,10 @@
* read.c: Clean this junk up! This is horrible code.
* Terminal read functions
*/
+#include <sys/ioctl.h>
+
#include <ctype.h>
#include <errno.h>
-#include <fcntl.h>
#include <limits.h>
#include <stdlib.h>
#include <string.h>
@@ -134,55 +135,16 @@ el_read_getfn(struct el_read_t *el_read)
/* read__fixio():
* Try to recover from a read error
*/
-/* ARGSUSED */
static int
-read__fixio(int fd __attribute__((__unused__)), int e)
+read__fixio(int fd, int e)
{
+ int zero = 0;
switch (e) {
- case -1: /* Make sure that the code is reachable */
-
-#ifdef EWOULDBLOCK
- case EWOULDBLOCK:
-#ifndef TRY_AGAIN
-#define TRY_AGAIN
-#endif
-#endif /* EWOULDBLOCK */
-
-#if defined(POSIX) && defined(EAGAIN)
-#if defined(EWOULDBLOCK) && EWOULDBLOCK != EAGAIN
case EAGAIN:
-#ifndef TRY_AGAIN
-#define TRY_AGAIN
-#endif
-#endif /* EWOULDBLOCK && EWOULDBLOCK != EAGAIN */
-#endif /* POSIX && EAGAIN */
-
- e = 0;
-#ifdef TRY_AGAIN
-#if defined(F_SETFL) && defined(O_NDELAY)
- if ((e = fcntl(fd, F_GETFL)) == -1)
+ if (ioctl(fd, FIONBIO, &zero) == -1)
return -1;
-
- if (fcntl(fd, F_SETFL, e & ~O_NDELAY) == -1)
- return -1;
- else
- e = 1;
-#endif /* F_SETFL && O_NDELAY */
-
-#ifdef FIONBIO
- {
- int zero = 0;
-
- if (ioctl(fd, FIONBIO, &zero) == -1)
- return -1;
- else
- e = 1;
- }
-#endif /* FIONBIO */
-
-#endif /* TRY_AGAIN */
- return e ? 0 : -1;
+ return 0;
case EINTR:
return 0;
