NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: PR/46935 CVS commit: src/lib/libedit



christos%zoulas.com@localhost (Christos Zoulas) wrote:

 |The following reply was made to PR lib/46935; it has been noted by GNATS.
 |
 |From: christos%zoulas.com@localhost (Christos Zoulas)
 |To: Steffen "Daode" Nurpmeso <sdaoden%gmail.com@localhost>, 
gnats-bugs%NetBSD.org@localhost
 |Cc: netbsd-bugs%netbsd.org@localhost, lib-bug-people%netbsd.org@localhost, 
 |      gnats-admin%netbsd.org@localhost
 |Subject: Re: PR/46935 CVS commit: src/lib/libedit
 |Date: Tue, 11 Sep 2012 09:41:43 -0400
 |
 | On Sep 11,  2:59pm, sdaoden%gmail.com@localhost (Steffen "Daode" Nurpmeso) 
wrote:
 | -- Subject: Re: PR/46935 CVS commit: src/lib/libedit
 | 
 || Steffen "Daode" Nurpmeso <sdaoden%gmail.com@localhost> wrote:
 || 
 ||  |The following reply was made to PR lib/46935; it has been noted by \
 |. GNATS.
 ||  [.]
 ||  |  | That is supposed to return a CMD; shouldn't it return \
 |. ED_END_OF_FILE or
 ||  |  | something?
 ||  |  | 
 ||  |  | christos
 ||  | 
 ||  | You're right, simply returning some value !OKCMD seems to be
 ||  | sufficient.
 || 
 || Uuh, no, it's much more complicated than that.
 | 
 | Ok, let me think about it then :-)
 | 
 | christos

I have this version running (on FreeBSD..) at the moment, and it
seems to work (i'm having READ_RESTART set here - any chance on
that?).

First el_getc() now *always* sets el_errno so that read_getcmd()
can definitely avoid doing so.

I think v1.69 still has the problem that the loop requires a new
stop condition (*num* is looked at in the switch, though the value
is effectively somewhat meaningless).
So i've introduced a new command state enum which should get
rid of any ambiguities that may arise from missing the complete
picture :), and which is used as a break-off command in the loop.

Note that i'm not sure at all about the CC_EOF case; it yet looked
at num==-1, but how can that happen there?  Anyway - no more.

This patch applies to v1.69 only with fuzziness, but it is a bit
chaotic here at the moment..

What do you think about that?

--steffen

--- read.c.orig 2012-09-11 15:51:03.000000000 +0200
+++ read.c      2012-09-11 15:51:03.000000000 +0200
@@ -52,13 +52,17 @@ __RCSID("$NetBSD: read.c,v 1.69 2012/09/
 #include <limits.h>
 #include "el.h"
 
-#define OKCMD  -1      /* must be -1! */
-
-private int    read__fixio(int, int);
-private int    read_preread(EditLine *);
-private int    read_char(EditLine *, Char *);
-private int    read_getcmd(EditLine *, el_action_t *, Char *);
-private void   read_pop(c_macro_t *);
+enum rcmd {
+       OKCMD   = -1,
+       EOFCMD  = 0,
+       ERRCMD  = 1
+};
+
+private int            read__fixio(int, int);
+private int            read_preread(EditLine *);
+private int            read_char(EditLine *, Char *);
+private enum rcmd      read_getcmd(EditLine *, el_action_t *, Char *);
+private void           read_pop(c_macro_t *);
 
 /* read_init():
  *     Initialize the read stuff
@@ -233,10 +237,10 @@ FUN(el,push)(EditLine *el, const Char *s
 
 
 /* read_getcmd():
- *     Get next command from the input stream, return OKCMD on success.
+ *     Get next command from the input stream.
  *     Character values > 255 are not looked up in the map, but inserted.
  */
-private int
+private enum rcmd
 read_getcmd(EditLine *el, el_action_t *cmdnum, Char *ch)
 {
        el_action_t cmd;
@@ -245,8 +249,7 @@ read_getcmd(EditLine *el, el_action_t *c
        el->el_errno = 0;
        do {
                if ((num = FUN(el,getc)(el, ch)) != 1) {/* if EOF or error */
-                       el->el_errno = num == 0 ? 0 : errno;
-                       return 0;       /* not OKCMD */
+                       return (num < 0 ? ERRCMD : EOFCMD);
                }
 
 #ifdef KANJI
@@ -430,14 +435,13 @@ FUN(el,getc)(EditLine *el, Char *cp)
        (void) fprintf(el->el_errfile, "Reading a character\n");
 #endif /* DEBUG_READ */
        num_read = (*el->el_read.read_char)(el, cp);
-       if (num_read < 0)
-               el->el_errno = errno;
+       el->el_errno = (num_read < 0) ? errno : 0;
 #ifdef WIDECHAR
        if (el->el_flags & NARROW_READ)
                *cp = *(char *)(void *)cp;
 #endif
 #ifdef DEBUG_READ
-       (void) fprintf(el->el_errfile, "Got it %c\n", *cp);
+       (void) fprintf(el->el_errfile, "Got <%c> (return %d)\n", *cp, num_read);
 #endif /* DEBUG_READ */
        return num_read;
 }
@@ -570,26 +574,32 @@ FUN(el,gets)(EditLine *el, int *nread)
                goto noedit;
        }
 
-       for (num = OKCMD; num == OKCMD;) {      /* while still editing this
-                                                * line */
+       /* While still editing this line */
+       for (num = 0;; num = 0) {
+               enum rcmd rcmd;
 #ifdef DEBUG_EDIT
                read_debug(el);
 #endif /* DEBUG_EDIT */
-               /* if EOF or error */
-               if ((num = read_getcmd(el, &cmdnum, &ch)) != OKCMD) {
-                       num = -1;
+               if ((rcmd = read_getcmd(el, &cmdnum, &ch)) != OKCMD) {
+                       if (rcmd == ERRCMD) {
+                               num = -1;
+                               if (el->el_errno == EINTR) {
+#ifdef DEBUG_READ
+                                       (void) fprintf(el->el_errfile,
+                                           "Returning from el_gets due to 
EINTR\n");
+#endif /* DEBUG_READ */
+                                       el->el_line.buffer[0] = '\0';
+                                       el->el_line.lastchar =
+                                           el->el_line.cursor = 
el->el_line.buffer;
+                                       break;
+                               }
+                       }
 #ifdef DEBUG_READ
                        (void) fprintf(el->el_errfile,
                            "Returning from el_gets %d\n", num);
 #endif /* DEBUG_READ */
                        break;
                }
-               if (el->el_errno == EINTR) {
-                       el->el_line.buffer[0] = '\0';
-                       el->el_line.lastchar =
-                           el->el_line.cursor = el->el_line.buffer;
-                       break;
-               }
                if ((unsigned int)cmdnum >= (unsigned int)el->el_map.nfunc) {   
/* BUG CHECK command */
 #ifdef DEBUG_EDIT
                        (void) fprintf(el->el_errfile,
@@ -662,9 +672,10 @@ FUN(el,gets)(EditLine *el, int *nread)
                        continue;       /* keep going... */
 
                case CC_EOF:    /* end of file typed */
+                       rcmd = EOFCMD;
                        if ((el->el_flags & UNBUFFERED) == 0)
                                num = 0;
-                       else if (num == -1) {
+                       else {
                                *el->el_line.lastchar++ = CONTROL('d');
                                el->el_line.cursor = el->el_line.lastchar;
                                num = 1;
@@ -672,6 +683,7 @@ FUN(el,gets)(EditLine *el, int *nread)
                        break;
 
                case CC_NEWLINE:        /* normal end of line */
+                       rcmd = EOFCMD;
                        num = (int)(el->el_line.lastchar - el->el_line.buffer);
                        break;

@@ -711,6 +711,8 @@ FUN(el,gets)(EditLine *el, int *nread)
                el->el_chared.c_vcmd.action = NOP;
                if (el->el_flags & UNBUFFERED)
                        break;
+               if (rcmd != OKCMD)
+                       break;
        }
 
        terminal__flush(el);            /* flush any buffered output */



Home | Main Index | Thread Index | Old Index