NetBSD Problem Report #48821
From yasuoka@iij.ad.jp Mon May 19 04:20:09 2014
Return-Path: <yasuoka@iij.ad.jp>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
(using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 81A69A580C
for <gnats-bugs@gnats.NetBSD.org>; Mon, 19 May 2014 04:20:09 +0000 (UTC)
Message-Id: <20140519.131957.1439305190873979253.yasuoka@iij.ad.jp>
Date: Mon, 19 May 2014 13:19:57 +0900 (JST)
From: yasuoka@iij.ad.jp
Reply-To: yasuoka@iij.ad.jp
To: gnats-bugs@gnats.NetBSD.org
Subject: libedit EL_SETTY doesn't work
X-Send-Pr-Version: 3.95
>Number: 48821
>Category: lib
>Synopsis: libedit EL_SETTY doesn't work
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: lib-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon May 19 04:25:00 +0000 2014
>Last-Modified: Wed May 21 13:10:01 +0000 2014
>Originator: yasuoka@iij.ad.jp
>Release: NetBSD 5.1.2
>Organization:
Internet Initiative Japan
>Environment:
System: NetBSD yasuoka-nb.tokyo.iiji.jp 5.1.2 NetBSD 5.1.2 (GENERIC) #0: Thu Feb 2 12:12:28 UTC 2012 builds@b7.netbsd.org:/home/builds/ab/netbsd-5-1-2-RELEASE/amd64/201202021012Z-obj/home/bui
lds/ab/netbsd-5-1-2-RELEASE/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
As the man page of editline(3) and editrc(5), the parameter "EL_SETTY"
can configure the terminal setting. By using this parameter, program
must be able to setup -ISIG not to receive SIGINT by Ctrl-C like
followings:
el_set(el, EL_SETTY, "-d", "-isig", NULL);
but this doesn't work.
In tty_rawmode() of lib/libedit/tty.c, the library seems to skip
applying the clear/set flag bits from "EL_SETTY" if the current
termios flags is not changed from cached flags.
In lib/libedit/tty.c:
991 if ((el->el_tty.t_ts.c_lflag != el->el_tty.t_ex.c_lflag) &&
992 (el->el_tty.t_ts.c_lflag != el->el_tty.t_ed.c_lflag)) {
Only if flags are changed from the cached flags,
993 el->el_tty.t_ex.c_lflag =
994 el->el_tty.t_ts.c_lflag;
995 el->el_tty.t_ex.c_lflag &=
996 ~el->el_tty.t_t[EX_IO][MD_LIN].t_clrmask;
997 el->el_tty.t_ex.c_lflag |=
998 el->el_tty.t_t[EX_IO][MD_LIN].t_setmask;
the {set,clr}masks come from "EL_SETTY" are applied. If any mask is
changed by "EL_SETTY", it must be applied.
>How-To-Repeat:
1. compile "libedittest.c" below
2. execute libedittest
3. enter Ctrl-C
-> the program will be interrupted
*** start libedittest.c ***
/* usage: cc -o libedittest libedittest.c -ledit -ltermcap */
#include <err.h>
#include <histedit.h>
#include <stdlib.h>
#include <termios.h>
static const char * prompt (EditLine *);
int
main(int argc, char *argv[])
{
extern char *__progname;
int num;
EditLine *el;
const char *line;
el = el_init(__progname, stdin, stdout, stderr);
el_set(el, EL_PROMPT, prompt);
el_set(el, EL_EDITOR, "emacs");
el_set(el, EL_SETTY, "-d", "-isig", NULL);
for (;;) {
line = el_gets(el, &num);
if (line == NULL)
break;
}
el_end(el);
exit(EXIT_SUCCESS);
}
static const char *
prompt(EditLine *el)
{
return "> ";
}
*** end libedittest.c ***
>Fix:
Index: tty.c
===================================================================
RCS file: /var/cvs/netbsd/src/lib/libedit/tty.c,v
retrieving revision 1.27
diff -u -p -r1.27 tty.c
--- tty.c 10 Sep 2008 15:45:37 -0000 1.27
+++ tty.c 19 May 2014 03:41:49 -0000
@@ -456,6 +456,7 @@ private void tty__getchar(struct termios
private void tty__setchar(struct termios *, unsigned char *);
private speed_t tty__getspeed(struct termios *);
private int tty_setup(EditLine *);
+private void tty_setup_flags(EditLine *, struct termios *, int);
#define t_qu t_ts
@@ -507,17 +508,7 @@ tty_setup(EditLine *el)
el->el_tty.t_tabs = tty__gettabs(&el->el_tty.t_ex);
el->el_tty.t_eight = tty__geteightbit(&el->el_tty.t_ex);
- el->el_tty.t_ex.c_iflag &= ~el->el_tty.t_t[EX_IO][MD_INP].t_clrmask;
- el->el_tty.t_ex.c_iflag |= el->el_tty.t_t[EX_IO][MD_INP].t_setmask;
-
- el->el_tty.t_ex.c_oflag &= ~el->el_tty.t_t[EX_IO][MD_OUT].t_clrmask;
- el->el_tty.t_ex.c_oflag |= el->el_tty.t_t[EX_IO][MD_OUT].t_setmask;
-
- el->el_tty.t_ex.c_cflag &= ~el->el_tty.t_t[EX_IO][MD_CTL].t_clrmask;
- el->el_tty.t_ex.c_cflag |= el->el_tty.t_t[EX_IO][MD_CTL].t_setmask;
-
- el->el_tty.t_ex.c_lflag &= ~el->el_tty.t_t[EX_IO][MD_LIN].t_clrmask;
- el->el_tty.t_ex.c_lflag |= el->el_tty.t_t[EX_IO][MD_LIN].t_setmask;
+ tty_setup_flags(el, &el->el_tty.t_ex, EX_IO);
/*
* Reset the tty chars to reasonable defaults
@@ -557,17 +548,7 @@ tty_setup(EditLine *el)
tty__setchar(&el->el_tty.t_ex, el->el_tty.t_c[EX_IO]);
#endif
- el->el_tty.t_ed.c_iflag &= ~el->el_tty.t_t[ED_IO][MD_INP].t_clrmask;
- el->el_tty.t_ed.c_iflag |= el->el_tty.t_t[ED_IO][MD_INP].t_setmask;
-
- el->el_tty.t_ed.c_oflag &= ~el->el_tty.t_t[ED_IO][MD_OUT].t_clrmask;
- el->el_tty.t_ed.c_oflag |= el->el_tty.t_t[ED_IO][MD_OUT].t_setmask;
-
- el->el_tty.t_ed.c_cflag &= ~el->el_tty.t_t[ED_IO][MD_CTL].t_clrmask;
- el->el_tty.t_ed.c_cflag |= el->el_tty.t_t[ED_IO][MD_CTL].t_setmask;
-
- el->el_tty.t_ed.c_lflag &= ~el->el_tty.t_t[ED_IO][MD_LIN].t_clrmask;
- el->el_tty.t_ed.c_lflag |= el->el_tty.t_t[ED_IO][MD_LIN].t_setmask;
+ tty_setup_flags(el, &el->el_tty.t_ed, ED_IO);
tty__setchar(&el->el_tty.t_ed, el->el_tty.t_c[ED_IO]);
tty_bind_char(el, 1);
@@ -964,69 +945,17 @@ tty_rawmode(EditLine *el)
(void) cfsetospeed(&el->el_tty.t_ed, el->el_tty.t_speed);
}
if (tty__cooked_mode(&el->el_tty.t_ts)) {
- if (el->el_tty.t_ts.c_cflag != el->el_tty.t_ex.c_cflag) {
- el->el_tty.t_ex.c_cflag =
- el->el_tty.t_ts.c_cflag;
- el->el_tty.t_ex.c_cflag &=
- ~el->el_tty.t_t[EX_IO][MD_CTL].t_clrmask;
- el->el_tty.t_ex.c_cflag |=
- el->el_tty.t_t[EX_IO][MD_CTL].t_setmask;
-
- el->el_tty.t_ed.c_cflag =
- el->el_tty.t_ts.c_cflag;
- el->el_tty.t_ed.c_cflag &=
- ~el->el_tty.t_t[ED_IO][MD_CTL].t_clrmask;
- el->el_tty.t_ed.c_cflag |=
- el->el_tty.t_t[ED_IO][MD_CTL].t_setmask;
- }
- if ((el->el_tty.t_ts.c_lflag != el->el_tty.t_ex.c_lflag) &&
- (el->el_tty.t_ts.c_lflag != el->el_tty.t_ed.c_lflag)) {
- el->el_tty.t_ex.c_lflag =
- el->el_tty.t_ts.c_lflag;
- el->el_tty.t_ex.c_lflag &=
- ~el->el_tty.t_t[EX_IO][MD_LIN].t_clrmask;
- el->el_tty.t_ex.c_lflag |=
- el->el_tty.t_t[EX_IO][MD_LIN].t_setmask;
-
- el->el_tty.t_ed.c_lflag =
- el->el_tty.t_ts.c_lflag;
- el->el_tty.t_ed.c_lflag &=
- ~el->el_tty.t_t[ED_IO][MD_LIN].t_clrmask;
- el->el_tty.t_ed.c_lflag |=
- el->el_tty.t_t[ED_IO][MD_LIN].t_setmask;
- }
- if ((el->el_tty.t_ts.c_iflag != el->el_tty.t_ex.c_iflag) &&
- (el->el_tty.t_ts.c_iflag != el->el_tty.t_ed.c_iflag)) {
- el->el_tty.t_ex.c_iflag =
- el->el_tty.t_ts.c_iflag;
- el->el_tty.t_ex.c_iflag &=
- ~el->el_tty.t_t[EX_IO][MD_INP].t_clrmask;
- el->el_tty.t_ex.c_iflag |=
- el->el_tty.t_t[EX_IO][MD_INP].t_setmask;
-
- el->el_tty.t_ed.c_iflag =
- el->el_tty.t_ts.c_iflag;
- el->el_tty.t_ed.c_iflag &=
- ~el->el_tty.t_t[ED_IO][MD_INP].t_clrmask;
- el->el_tty.t_ed.c_iflag |=
- el->el_tty.t_t[ED_IO][MD_INP].t_setmask;
- }
- if ((el->el_tty.t_ts.c_oflag != el->el_tty.t_ex.c_oflag) &&
- (el->el_tty.t_ts.c_oflag != el->el_tty.t_ed.c_oflag)) {
- el->el_tty.t_ex.c_oflag =
- el->el_tty.t_ts.c_oflag;
- el->el_tty.t_ex.c_oflag &=
- ~el->el_tty.t_t[EX_IO][MD_OUT].t_clrmask;
- el->el_tty.t_ex.c_oflag |=
- el->el_tty.t_t[EX_IO][MD_OUT].t_setmask;
-
- el->el_tty.t_ed.c_oflag =
- el->el_tty.t_ts.c_oflag;
- el->el_tty.t_ed.c_oflag &=
- ~el->el_tty.t_t[ED_IO][MD_OUT].t_clrmask;
- el->el_tty.t_ed.c_oflag |=
- el->el_tty.t_t[ED_IO][MD_OUT].t_setmask;
- }
+ el->el_tty.t_ed.c_iflag = el->el_tty.t_ex.c_iflag =
+ el->el_tty.t_ts.c_iflag;
+ el->el_tty.t_ed.c_oflag = el->el_tty.t_ex.c_oflag =
+ el->el_tty.t_ts.c_oflag;
+ el->el_tty.t_ed.c_cflag = el->el_tty.t_ex.c_cflag =
+ el->el_tty.t_ts.c_cflag;
+ el->el_tty.t_ed.c_lflag = el->el_tty.t_ex.c_lflag =
+ el->el_tty.t_ts.c_lflag;
+ tty_setup_flags(el, &el->el_tty.t_ex, EX_IO);
+ tty_setup_flags(el, &el->el_tty.t_ed, ED_IO);
+
if (tty__gettabs(&el->el_tty.t_ex) == 0)
el->el_tty.t_tabs = 0;
else
@@ -1120,18 +1049,7 @@ tty_quotemode(EditLine *el)
return (0);
el->el_tty.t_qu = el->el_tty.t_ed;
-
- el->el_tty.t_qu.c_iflag &= ~el->el_tty.t_t[QU_IO][MD_INP].t_clrmask;
- el->el_tty.t_qu.c_iflag |= el->el_tty.t_t[QU_IO][MD_INP].t_setmask;
-
- el->el_tty.t_qu.c_oflag &= ~el->el_tty.t_t[QU_IO][MD_OUT].t_clrmask;
- el->el_tty.t_qu.c_oflag |= el->el_tty.t_t[QU_IO][MD_OUT].t_setmask;
-
- el->el_tty.t_qu.c_cflag &= ~el->el_tty.t_t[QU_IO][MD_CTL].t_clrmask;
- el->el_tty.t_qu.c_cflag |= el->el_tty.t_t[QU_IO][MD_CTL].t_setmask;
-
- el->el_tty.t_qu.c_lflag &= ~el->el_tty.t_t[QU_IO][MD_LIN].t_clrmask;
- el->el_tty.t_qu.c_lflag |= el->el_tty.t_t[QU_IO][MD_LIN].t_setmask;
+ tty_setup_flags(el, &el->el_tty.t_qu, QU_IO);
if (tty_setty(el, TCSADRAIN, &el->el_tty.t_qu) == -1) {
#ifdef DEBUG_TTY
@@ -1343,3 +1261,19 @@ tty_printchar(EditLine *el, unsigned cha
(void) fprintf(el->el_errfile, "\n");
}
#endif /* notyet */
+
+private void
+tty_setup_flags(EditLine *el, struct termios *tios, int mode)
+{
+ tios->c_iflag &= ~el->el_tty.t_t[mode][MD_INP].t_clrmask;
+ tios->c_iflag |= el->el_tty.t_t[mode][MD_INP].t_setmask;
+
+ tios->c_oflag &= ~el->el_tty.t_t[mode][MD_OUT].t_clrmask;
+ tios->c_oflag |= el->el_tty.t_t[mode][MD_OUT].t_setmask;
+
+ tios->c_cflag &= ~el->el_tty.t_t[mode][MD_CTL].t_clrmask;
+ tios->c_cflag |= el->el_tty.t_t[mode][MD_CTL].t_setmask;
+
+ tios->c_lflag &= ~el->el_tty.t_t[mode][MD_LIN].t_clrmask;
+ tios->c_lflag |= el->el_tty.t_t[mode][MD_LIN].t_setmask;
+}
>Audit-Trail:
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: lib/48821: libedit EL_SETTY doesn't work
Date: Mon, 19 May 2014 09:47:16 -0400
On May 19, 4:25am, yasuoka@iij.ad.jp (yasuoka@iij.ad.jp) wrote:
-- Subject: lib/48821: libedit EL_SETTY doesn't work
| >Number: 48821
| >Category: lib
| >Synopsis: libedit EL_SETTY doesn't work
| >Confidential: no
| >Severity: non-critical
| >Priority: medium
| >Responsible: lib-bug-people
| >State: open
| >Class: sw-bug
| >Submitter-Id: net
| >Arrival-Date: Mon May 19 04:25:00 +0000 2014
| >Originator: yasuoka@iij.ad.jp
| >Release: NetBSD 5.1.2
| >Organization:
| Internet Initiative Japan
| >Environment:
| System: NetBSD yasuoka-nb.tokyo.iiji.jp 5.1.2 NetBSD 5.1.2 (GENERIC) #0: Thu Feb 2 12:12:28 UTC 2012 builds@b7.netbsd.org:/home/builds/ab/netbsd-5-1-2-RELEASE/amd64/201202021012Z-obj/home/bui
| lds/ab/netbsd-5-1-2-RELEASE/src/sys/arch/amd64/compile/GENERIC amd64
| Architecture: x86_64
| Machine: amd64
| >Description:
|
| As the man page of editline(3) and editrc(5), the parameter "EL_SETTY"
| can configure the terminal setting. By using this parameter, program
| must be able to setup -ISIG not to receive SIGINT by Ctrl-C like
| followings:
|
| el_set(el, EL_SETTY, "-d", "-isig", NULL);
|
| but this doesn't work.
|
| In tty_rawmode() of lib/libedit/tty.c, the library seems to skip
| applying the clear/set flag bits from "EL_SETTY" if the current
| termios flags is not changed from cached flags.
While I like the refactoring of all tis duplicated code, the removal
of the conditional setting is undesirable. It is meant to prevent the
user for altering the state of the tty from a 3rd party. Consider using
the editor in the shell (which is used in /bin/sh). vi crashes and leaves
the tty in non-canonical, no echo mode. With your change, the changes
from vi get immediately reflected in the editor. What we want instead,
is to recognize the settings from setty as valid, and reflect those
unconditionally. Perhaps that code is broken.
christos
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/48821 CVS commit: src/lib/libedit
Date: Mon, 19 May 2014 13:14:41 -0400
Module Name: src
Committed By: christos
Date: Mon May 19 17:14:41 UTC 2014
Modified Files:
src/lib/libedit: tty.c
Log Message:
Factor out some common code (more to be done) from PR/48821
To generate a diff of this commit:
cvs rdiff -u -r1.42 -r1.43 src/lib/libedit/tty.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/48821 CVS commit: src/lib/libedit
Date: Mon, 19 May 2014 17:01:48 -0400
Module Name: src
Committed By: christos
Date: Mon May 19 21:01:48 UTC 2014
Modified Files:
src/lib/libedit: tty.c
Log Message:
PR/48821: If called from tty_stty(), recalculate flags.
To generate a diff of this commit:
cvs rdiff -u -r1.44 -r1.45 src/lib/libedit/tty.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: YASUOKA Masahiko <yasuoka@iij.ad.jp>
To: christos@zoulas.com, gnats-bugs@NetBSD.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/48821: libedit EL_SETTY doesn't work
Date: Wed, 21 May 2014 17:55:31 +0900 (JST)
On Mon, 19 May 2014 13:50:01 +0000 (UTC)
christos@zoulas.com (Christos Zoulas) wrote:
> While I like the refactoring of all tis duplicated code, the removal
> of the conditional setting is undesirable.
I agree keeping the condition is safer, but I still cann't understand
what does the condition mean.
> It is meant to prevent the user for altering the state of the tty
> from a 3rd party. Consider using the editor in the shell (which is
> used in /bin/sh). vi crashes and leaves the tty in non-canonical, no
> echo mode.
Yes,
> With your change, the changes from vi get immediately reflected in
> the editor.
The code can be simplified like below:
tcsetattr(&ts);
// ts = the current setting
// ex = last termios for cooked mode
// ed = last termios for rawmode
if (ts != ex && ts != ed) {// the settings is changed by a 3rd party
ex = ed = ts; // adapt the change from the 3rd party
ex |= setbits; // adapt restriction from SETTY
ex &= ~clrbits; // same as above
ed |= setbits; // same as above
ed &= ~clrbits; // same as above
}
tcsetattr(&ed); // reflect
My change was to remove the "if (..." condition. So the removal of
the condition doesn't alter the behavior of "the changes from vi"
case.
But it alters the case in which the tty setting isn't changed. In
that case, I think executing the conditional block isn't a problem,
since 'ts' is clean (because it's unchanged) and adapting the
restriction should be ok.
--yasuoka
From: YASUOKA Masahiko <yasuoka@iij.ad.jp>
To: christos@zoulas.com, gnats-bugs@NetBSD.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/48821: libedit EL_SETTY doesn't work
Date: Wed, 21 May 2014 17:57:33 +0900 (JST)
On Wed, 21 May 2014 17:55:31 +0900 (JST)
YASUOKA Masahiko <yasuoka@iij.ad.jp> wrote:
> On Mon, 19 May 2014 13:50:01 +0000 (UTC)
> christos@zoulas.com (Christos Zoulas) wrote:
>> While I like the refactoring of all tis duplicated code, the removal
>> of the conditional setting is undesirable.
>
> I agree keeping the condition is safer, but I still cann't understand
> what does the condition mean.
>
>> It is meant to prevent the user for altering the state of the tty
>> from a 3rd party. Consider using the editor in the shell (which is
>> used in /bin/sh). vi crashes and leaves the tty in non-canonical, no
>> echo mode.
>
> Yes,
>
>> With your change, the changes from vi get immediately reflected in
>> the editor.
>
> The code can be simplified like below:
>
> tcsetattr(&ts);
sorry, this should be
tcgetattr(&ts);
> // ts = the current setting
> // ex = last termios for cooked mode
> // ed = last termios for rawmode
>
> if (ts != ex && ts != ed) {// the settings is changed by a 3rd party
> ex = ed = ts; // adapt the change from the 3rd party
> ex |= setbits; // adapt restriction from SETTY
> ex &= ~clrbits; // same as above
> ed |= setbits; // same as above
> ed &= ~clrbits; // same as above
> }
> tcsetattr(&ed); // reflect
>
> My change was to remove the "if (..." condition. So the removal of
> the condition doesn't alter the behavior of "the changes from vi"
> case.
>
> But it alters the case in which the tty setting isn't changed. In
> that case, I think executing the conditional block isn't a problem,
> since 'ts' is clean (because it's unchanged) and adapting the
> restriction should be ok.
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, yasuoka@iij.ad.jp
Cc:
Subject: Re: lib/48821: libedit EL_SETTY doesn't work
Date: Wed, 21 May 2014 09:08:32 -0400
On May 21, 9:00am, yasuoka@iij.ad.jp (YASUOKA Masahiko) wrote:
-- Subject: Re: lib/48821: libedit EL_SETTY doesn't work
| The code can be simplified like below:
|
| tcsetattr(&ts);
| // ts = the current setting
| // ex = last termios for cooked mode
| // ed = last termios for rawmode
|
| if (ts != ex && ts != ed) {// the settings is changed by a 3rd party
| ex = ed = ts; // adapt the change from the 3rd party
| ex |= setbits; // adapt restriction from SETTY
| ex &= ~clrbits; // same as above
| ed |= setbits; // same as above
| ed &= ~clrbits; // same as above
| }
| tcsetattr(&ed); // reflect
|
| My change was to remove the "if (..." condition. So the removal of
| the condition doesn't alter the behavior of "the changes from vi"
| case.
Right, now that I think about it, the change is perhaps just an optimization
(to execute less code if something did not change). It's been such a long
time since I wrote the code and so many fingers have touched it.
christos
(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-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.