NetBSD Problem Report #48957

From www@NetBSD.org  Mon Jun 30 20:26:13 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C91D5A585B
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 30 Jun 2014 20:26:13 +0000 (UTC)
Message-Id: <20140630202612.60DD5A6545@mollari.NetBSD.org>
Date: Mon, 30 Jun 2014 20:26:12 +0000 (UTC)
From: fgsch@lodoss.net
Reply-To: fgsch@lodoss.net
To: gnats-bugs@NetBSD.org
Subject: Broken rl_callback_read_char() in libedit's readline emulation
X-Send-Pr-Version: www-1.0

>Number:         48957
>Category:       lib
>Synopsis:       Broken rl_callback_read_char() in libedit's readline emulation
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 30 20:30:00 +0000 2014
>Closed-Date:    Fri Dec 08 19:55:41 +0000 2017
>Last-Modified:  Fri Dec 08 19:55:41 +0000 2017
>Originator:     Federico G. Schwindt
>Release:        libedit
>Organization:
>Environment:
other
>Description:
readline.c r1.85 introduced, amongst lot of changes, this:

-		el_set(e, EL_UNBUFFERED, 1);
+		//el_set(e, EL_UNBUFFERED, 1);

Unfortunately it's not clear why was this done as the commit message only says:

apply apple patches from:
http://opensource.apple.com/source/libedit/libedit-11/patches/

This breaks programs installing their own handler via rl_callback_handler_install() and using rl_callback_read_char() since the line buffer is not reset anymore.
>How-To-Repeat:
Testcase below.
This works fine with gnu readline and uncommenting the line previously mentioned.

#include <editline/readline.h>
#include <poll.h>
#include <stdlib.h>
#include <unistd.h>

static int refresh = 0;

void
fn_handler(char *l)
{
        if (l)
                refresh = 1;
}

int
main()
{
        struct pollfd fds[1];
        int i;

        rl_already_prompted = 1;
        rl_callback_handler_install("prompt> ", fn_handler);
        fds[0].fd = 0;
        fds[0].events = POLLIN;

        for (;;) {
                i = poll(fds, 1, 50);
                if (refresh) {
                        refresh = 0;
                        write(1, "\r           \r", 13);
                        rl_forced_update_display();
                }
                if (fds[0].revents & POLLIN)
                        rl_callback_read_char();
        }
        exit(0);
}
>Fix:
Remove comment from rl_callback_read_char(). Alternatively ensure line buffer is reset correctly after the callback is invoked.

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48957 CVS commit: src/lib/libedit
Date: Sun, 6 Jul 2014 14:09:04 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sun Jul  6 18:09:04 UTC 2014

 Modified Files:
 	src/lib/libedit: readline.c

 Log Message:
 PR/48957: Federico G. Schwindt: Restore commented out code that broke
 rl_callback_handler.


 To generate a diff of this commit:
 cvs rdiff -u -r1.110 -r1.111 src/lib/libedit/readline.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Simon J. Gerraty" <sjg@juniper.net>
To: <christos@netbsd.org>
Cc: <sjg@juniper.net>, <fgsch@lodoss.net>, <gnats-bugs@netbsd.org>
Subject: Re: lib/48957 libedit/readline.c prompt bug
Date: Thu, 7 Dec 2017 17:01:48 -0800

 Hi Christos,

 > when we build python for Junos we use libedit's readline.
 > However there's a bug. We fixed it internally some years ago,
 > but it was never upstreamed.
 > 
 > Having just wasted several hours re-discoverying that, I figured
 > I'd address it.
 > 
 > Patch is below - it removes the call to set EL_UNBUFFERED
 > from rl_callback_read_char() since this causes the prompt to be issued
 > again - before caller has processed current input.

 So after more digging I see that this was previously commented out then
 uncommented in reponse to this PR 48957
 but the test app there appears to me to be questionable.

 If I tweak that to stop doing the
 write "\r                  \r" dance,
 (which is hiding the extra prompt output)
 we see that the bug is there:

 prompt> hi
 prompt> prompt> there
 prompt> prompt> oops
 prompt> prompt>

 as I said, I'm not certain what the best fix is, but
 calling el_set(e, EL_UNBUFFERED, 1) here does not appear to be correct.

From: "Simon J. Gerraty" <sjg@juniper.net>
To: Christos Zoulas <christos@zoulas.com>
Cc: <sjg@juniper.net>, <gnats-bugs@NetBSD.org>
Subject: Re: Re: lib/48957 libedit/readline.c prompt bug
Date: Thu, 7 Dec 2017 18:03:30 -0800

 Christos Zoulas <christos@zoulas.com> wrote:
 > The caller is supposed to deal with input inside rl_linefunc according
 > to this:
 > 
 > https://

 Sorry stupid IT filter renders the url useless ;-)

 > ...since the prompt gets printed before the read_char returns.
 > 
 > Do you have code that reproduces the different behavior between readline
 > and libedit? Because my example does not do it.

 Using straight readline isn't a problem, its code using the callback
 dance.

 Below is a simple test app, that captures the essence of what Python's
 readline extension does (started from cpp output from that ;-)

 When linked with real readline Python behaves ok.
 With libedit/readline it looks like:

 Python 2.7.8 (default, Dec  6 2017, 13:52:53)
 [GCC 4.2.1 Compatible Juniper Clang 3.7.1
 (git@psd-tools-git01.juniper.net:tool on junos
 Type "help", "copyright", "credits" or "license" for more information.

 >>> print 'hi'
 >>> hi
 print 'there'
 >>> there
 ^D

 which is broken.
 I added the original copyright back to keep our lawyers happy ;-)

 /*
  * This test case is derrived from pre-processed output of
  * python's readline extension - expecting to use gnu readline
  * so we keep the original copyright...
  * I have removed all the Py* and anything else that wasn't necessary
  * to reproduce the prompt bug (prompt being issued before
  * call_readline returns)
  * with libedit/readline as currently in NetBSD and FreeBSD we see:
  *
  * >>> print 'hi'
  * >>> hi
  * print 'there'
  * >>> there
  *
  * rather than:
  *
  * >>> print 'hi'
  * hi
  * >>> print 'there'
  * there
  * >>>
  */
 /* This module makes GNU readline available to Python.  It has ideas
  * contributed by Lee Busby, LLNL, and William Magro, Cornell Theory
  * Center.  The completer interface was inspired by Lele Gaifax.  More
  * recently, it was largely rewritten by Guido van Rossum.
  */

 #include <sys/select.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <edit/readline/readline.h>
 #include <edit/readline/history.h>

 static char *completer_word_break_characters;

 static int _history_length = -1;

 static void
 setup_readline(void)
 {
     using_history();

     rl_readline_name = "python";

     rl_bind_key('\t', rl_insert);

     rl_bind_key_in_map ('\t', rl_complete, emacs_meta_keymap);
     rl_bind_key_in_map ('\033', rl_complete, emacs_meta_keymap);

     completer_word_break_characters =
         rl_completer_word_break_characters =
         strdup(" \t\n`~!@#$%^&*()-=+[{]}\\|;:'\",<>/?");

         rl_initialize();
 }


 static char *completed_input_string;
 static void
 rlhandler(char *text)
 {
     completed_input_string = text;
     rl_callback_handler_remove();
 }

 static char *
 readline_until_enter_or_signal(char *prompt, int *signal)
 {
     char * not_done_reading = "";
     fd_set selectset;

     *signal = 0;

     rl_callback_handler_install (prompt, rlhandler);
     FD_ZERO(&selectset);

     completed_input_string = not_done_reading;

     while (completed_input_string == not_done_reading) {
         int has_input = 0;

         while (!has_input)
         { struct timeval timeout = {0, 100000};


 	    FD_SET(0, &selectset);
             has_input = select(2, &selectset, NULL, NULL, &timeout);
         }

         if(has_input > 0) {
             rl_callback_read_char();
         }
     }

     return completed_input_string;
 }

 static char *
 call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 {
     static int once;
     size_t n;
     char *p, *q;
     int signal;
     HIST_ENTRY *he;


     if (sys_stdin != rl_instream || sys_stdout != rl_outstream) {
         rl_instream = sys_stdin;
         rl_outstream = sys_stdout;

         rl_prep_terminal (1);

     }


     p = readline_until_enter_or_signal(prompt, &signal);


     if (signal) {
         return ((void *)0);
     }


     if (p == ((void *)0)) {
         return p;
     }


     n = strlen(p);
     return p;
 }



 int
 main(int argc, char *argv[])
 {
     char *cp;
     setup_readline();    
     while ((cp = call_readline(stdin, stdout, ">>> "))) {
 	printf("read: '%s'\n", cp);
     }
     return 0;
 }

From: "Simon J. Gerraty" <sjg@juniper.net>
To: <christos@netbsd.org>, <fgsch@lodoss.net>, <gnats-bugs@netbsd.org>,
        <sjg@juniper.net>
Cc: 
Subject: Re: lib/48957 libedit/readline.c prompt bug
Date: Thu, 7 Dec 2017 17:49:12 -0800

 Simon J. Gerraty <sjg@juniper.net> wrote:
 > If I tweak that to stop doing the
 > write "\r                  \r" dance,
 > (which is hiding the extra prompt output)
 > we see that the bug is there:
 > 
 > prompt> hi
 > prompt> prompt> there
 > prompt> prompt> oops
 > prompt> prompt>
 > 
 > as I said, I'm not certain what the best fix is, but
 > calling el_set(e, EL_UNBUFFERED, 1) here does not appear to be correct.

 With the patch below, python works correctly:

 Python 2.7.8 (default, Dec  6 2017, 13:52:53)
 [GCC 4.2.1 Compatible Juniper Clang 3.7.1
 (git@psd-tools-git01.juniper.net:tool on junos
 Type "help", "copyright", "credits" or "license" for more information.

 >>> print 'hi'
 hi
 >>> ^D

 and fgsch's test app appears to behave as well as it might (without the
 write dance hiding anything) and initializing rl_already_prompted=0
 which makes more sense since the app isn't issuing the prompt itself:

 prompt> hi
 prompt> hi there
 prompt>  there
 prompt>  there
 prompt>
 prompt> one



 Index: lib/libedit/readline.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libedit/readline.c,v
 retrieving revision 1.144
 diff -u -p -r1.144 readline.c
 --- lib/libedit/readline.c	17 Sep 2017 08:10:08 -0000	1.144
 +++ lib/libedit/readline.c	8 Dec 2017 01:39:24 -0000
 @@ -2079,7 +2079,10 @@ rl_callback_read_char(void)
  		} else
  			wbuf = NULL;
  		(*(void (*)(const char *))rl_linefunc)(wbuf);
 -		el_set(e, EL_UNBUFFERED, 1);
 +		if (!rl_already_prompted) {
 +		    el_set(e, EL_UNBUFFERED, 1);
 +		    rl_already_prompted = 1;
 +		}
  	}
  }


From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, fgsch@lodoss.net
Cc: 
Subject: Re: lib/48957 libedit/readline.c prompt bug
Date: Fri, 8 Dec 2017 11:56:46 -0500

 On Dec 8,  3:55am, sjg@juniper.net ("Simon J. Gerraty") wrote:
 -- Subject: Re: lib/48957 libedit/readline.c prompt bug

 Committed thanks!

 christos

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Fri, 08 Dec 2017 19:55:41 +0000
State-Changed-Why:
Fixed, thanks for the patches and follow up check!


>Unformatted:

Home	PR Database Search