Hi,

in its current state, the editline(3) library is completely unfit
to work with non-blocking I/O.  Neither the API nor the implementation
are even designed for that purpose.

I do have some candidate ideas to minimally extend the API - without
adding any new functions - to also work with non-blocking I/O, but
frankly, i don't see any sane use case for it yet.  Why would anyone
desire to mix non-blocking I/O and interactive line editing with the
editline(3) library on the same file descriptor?

Consequently, i propose that we document the facts up front and
simply delete some code that isn't going to do any good in practice.
For programs not using non-blocking I/O on the editline input file
descriptor, this patch makes no difference.  Programs that do set
FIONBIO on a file descriptor and then call editline(3) on it anyway
can't possibly work as intended as far as i can see.  Either setting
FIONBIO is needless in the first place, or el_gets(3) clearing it
will ruin the rest of the program's functionality.

Do any of you have any doubts, and if so, which ones?

Or is anybody willing to OK this patch?


If soembody comes up with a sane use case for mixing FIONBIO with
editline(3) at a later point in time, we can still consider my plans
to make that work.  In that case, deleting the junk deleted by this
patch would be required as the first step anyway.


I'm appending a simple test program at the end.

Before the patch:

  nbio is on
  ?test
  the user said: test
  nbio is off            /* oops, what is this? */

After the patch:

  nbio is on
  ?progname: el_gets: Resource temporarily unavailable

I think the latter makes more sense and is less surprising.

Yours,
  Ingo


Index: editline.3
===================================================================
RCS file: /cvs/src/lib/libedit/editline.3,v
retrieving revision 1.46
diff -u -p -r1.46 editline.3
--- editline.3  22 May 2016 22:08:42 -0000      1.46
+++ editline.3  11 Aug 2021 16:20:00 -0000
@@ -169,6 +169,20 @@ Programs should be linked with
 .Pp
 The
 .Nm
+library is designed to work with blocking I/O only.
+If the
+.Dv FIONBIO
+.Xr ioctl 2
+is set on
+.Ar fin ,
+.Fn el_gets
+and
+.Fn el_wgets
+will almost certainly fail with
+.Ar EAGAIN .
+.Pp
+The
+.Nm
 library respects the
 .Ev LC_CTYPE
 locale set by the application program and never uses
Index: read.c
===================================================================
RCS file: /cvs/src/lib/libedit/read.c,v
retrieving revision 1.47
diff -u -p -r1.47 read.c
--- read.c      11 Aug 2021 15:13:46 -0000      1.47
+++ read.c      11 Aug 2021 16:20:00 -0000
@@ -66,7 +66,6 @@ struct el_read_t {
        int              read_errno;
 };
 
-static int     read__fixio(int, int);
 static int     read_char(EditLine *, wchar_t *);
 static int     read_getcmd(EditLine *, el_action_t *, wchar_t *);
 static void    read_clearmacros(struct macros *);
@@ -132,26 +131,6 @@ el_read_getfn(struct el_read_t *el_read)
 }
 
 
-/* read__fixio():
- *     Try to recover from a read error
- */
-static int
-read__fixio(int fd, int e)
-{
-       int zero = 0;
-
-       switch (e) {
-       case EAGAIN:
-               if (ioctl(fd, FIONBIO, &zero) == -1)
-                       return -1;
-               return 0;
-
-       default:
-               return -1;
-       }
-}
-
-
 /* el_push():
  *     Push a macro
  */
@@ -231,15 +210,12 @@ static int
 read_char(EditLine *el, wchar_t *cp)
 {
        ssize_t num_read;
-       int tried = 0;
        char cbuf[MB_LEN_MAX];
        int cbp = 0;
-       int save_errno = errno;
 
  again:
        el->el_signal->sig_no = 0;
        while ((num_read = read(el->el_infd, cbuf + cbp, 1)) == -1) {
-               int e = errno;
                if (errno == EINTR) {
                        switch (el->el_signal->sig_no) {
                        case SIGCONT:
@@ -252,14 +228,8 @@ read_char(EditLine *el, wchar_t *cp)
                                break;
                        }
                }
-               if (!tried && read__fixio(el->el_infd, e) == 0) {
-                       errno = save_errno;
-                       tried = 1;
-               } else {
-                       errno = e;
-                       *cp = L'\0';
-                       return -1;
-               }
+               *cp = L'\0';
+               return -1;
        }
 
        /* Test for EOF */


 ----- 8< ----- schnipp ----- >8 ----- 8< ----- schnapp ----- >8 -----


#include <err.h>
#include <fcntl.h>
#include <histedit.h>
#include <stdio.h>
#include <unistd.h>

int
main(void)
{
        EditLine        *el;
        const char      *line;
        int              len;
        int              flags = O_NONBLOCK;

        if (fcntl(STDIN_FILENO, F_SETFL, &flags) == -1)
                err(1, "fcntl(F_SETFL)");
        flags = fcntl(STDIN_FILENO, F_GETFL);
        printf("nbio is %s\n", flags & O_NONBLOCK ? "on" : "off");
        if ((el = el_init("test", stdin, stdout, stderr)) == NULL)
                err(1, "el_init");
        if ((line = el_gets(el, &len)) == NULL)
                err(1, "el_gets");
        printf("the user said: %.*s", len, line);
        flags = fcntl(STDIN_FILENO, F_GETFL);
        printf("nbio is %s\n", flags & O_NONBLOCK ? "on" : "off");
        return 0;
}

Reply via email to