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:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.