NetBSD-Bugs archive

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

lib/52517: TTY settings modified and not restored on exit.



>Number:         52517
>Category:       lib
>Synopsis:       TTY settings modified and not restored on exit.
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 31 21:50:00 +0000 2017
>Originator:     Jay West
>Release:        libedit (NetBSD-current)
>Organization:
Self
>Environment:
Linux [deleted] 2.6.32-696.6.3.el6.x86_64 #1 SMP Wed Jul 12 14:17:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
>Description:
libedit modifies TTY settings when used, and does not have a mechanism to fully save and restore TTY settings.  This causes problems when libedit is used as a GNU readline replacement.

Specifically, after running python+GnuReadline, TTY settings are untouched after python exits.  However, python+libedit has noticeable TTY changes after python exits.

This will be true for any application linked against libedit instead of GnuReadline.
>How-To-Repeat:
% stty -a > orig.txt
% python # Run python compiled with libedit, and immediately exit (or any application using libedit).
% stty -a > after.txt
% diff orig.txt after.txt  # Differences exist

>Fix:
Patch to save TTY settings on init, and restore on program exit.
-------------------------
diff --git a/src/terminal.c b/src/terminal.c
index 26838e813d..53441ea6d8 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -268,6 +268,7 @@ terminal_setflags(EditLine *el)
 libedit_private int
 terminal_init(EditLine *el)
 {
+	tty_save();

 	el->el_terminal.t_buf = el_malloc(TC_BUFSIZE *
 	    sizeof(*el->el_terminal.t_buf));
diff --git a/src/tty.c b/src/tty.c
index f524da4734..e3adcdd659 100644
--- a/src/tty.c
+++ b/src/tty.c
@@ -467,6 +467,190 @@ static void	tty_setup_flags(EditLine *, struct termios *, int);

 #define	t_qu	t_ts

+/**********
+ * Acknowledgement:  tty_save/tty_restore/tty_signal is from
+ *      https://cboard.cprogramming.com/linux-programming/158476-termios-examples.html
+ **********/
+
+static int            tty_descriptor = -1;
+static struct termios tty_original;
+static struct termios tty_settings;
+
+struct termios* tty_get_original() {
+  return &tty_original;
+}
+
+/* Restore terminal to original settings
+*/
+static void tty_restore(void)
+{
+    if (tty_descriptor != -1)
+        tcsetattr(tty_descriptor, TCSANOW, &tty_original);
+}
+
+/* "Default" signal handler: restore terminal, then exit.
+*/
+static void tty_signal(int signum)
+{
+    if (tty_descriptor != -1)
+        tcsetattr(tty_descriptor, TCSANOW, &tty_original);
+
+    /* exit() is not async-signal safe, but _exit() is.
+     * Use the common idiom of 128 + signal number for signal exits.
+     * Alternative approach is to reset the signal to default handler,
+     * and immediately raise() it. */
+    _exit(128 + signum);
+}
+
+/* Initialize terminal for non-canonical, non-echo mode,
+ * that should be compatible with standard C I/O.
+ * Returns 0 if success, nonzero errno otherwise.
+*/
+libedit_private int
+tty_save(void)
+{
+    struct sigaction act;
+
+    /* Already initialized? */
+    if (tty_descriptor != -1)
+        return errno = 0;
+
+    /* Which standard stream is connected to our TTY? */
+    if (isatty(STDERR_FILENO))
+        tty_descriptor = STDERR_FILENO;
+    else
+    if (isatty(STDIN_FILENO))
+        tty_descriptor = STDIN_FILENO;
+    else
+    if (isatty(STDOUT_FILENO))
+        tty_descriptor = STDOUT_FILENO;
+    else
+        return errno = ENOTTY;
+
+    /* Obtain terminal settings. */
+    if (tcgetattr(tty_descriptor, &tty_original) ||
+        tcgetattr(tty_descriptor, &tty_settings))
+        return errno = ENOTSUP;
+
+    /* Disable buffering for terminal streams. */
+    /*
+    if (isatty(STDIN_FILENO))
+        setvbuf(stdin, NULL, _IONBF, 0);
+    if (isatty(STDOUT_FILENO))
+        setvbuf(stdout, NULL, _IONBF, 0);
+    if (isatty(STDERR_FILENO))
+        setvbuf(stderr, NULL, _IONBF, 0);
+    */
+
+    /* At exit() or return from main(),
+     * restore the original settings. */
+    if (atexit(tty_restore))
+        return errno = ENOTSUP;
+
+    /* Set new "default" handlers for typical signals,
+     * so that if this process is killed by a signal,
+     * the terminal settings will still be restored first. */
+    sigemptyset(&act.sa_mask);
+    act.sa_handler = tty_signal;
+    act.sa_flags = 0;
+    if (sigaction(SIGHUP,  &act, NULL) ||
+        sigaction(SIGINT,  &act, NULL) ||
+        sigaction(SIGQUIT, &act, NULL) ||
+        sigaction(SIGTERM, &act, NULL) ||
+#ifdef SIGXCPU
+        sigaction(SIGXCPU, &act, NULL) ||
+#endif
+#ifdef SIGXFSZ
+        sigaction(SIGXFSZ, &act, NULL) ||
+#endif
+#ifdef SIGIO
+        sigaction(SIGIO,   &act, NULL) ||
+#endif
+        sigaction(SIGPIPE, &act, NULL) ||
+        sigaction(SIGALRM, &act, NULL) ||
+        sigaction(SIGSEGV, &act, NULL) ||
+        sigaction(SIGABRT, &act, NULL))
+      return errno = ENOTSUP;
+
+    /* Let BREAK cause a SIGINT in input. */
+    /*
+    tty_settings.c_iflag &= ~IGNBRK;
+    tty_settings.c_iflag |=  BRKINT;
+    */
+
+    /* Ignore framing and parity errors in input. */
+    /*
+    tty_settings.c_iflag |=  IGNPAR;
+    tty_settings.c_iflag &= ~PARMRK;
+    */
+
+    /* Do not strip eighth bit on input. */
+    /*
+    tty_settings.c_iflag &= ~ISTRIP;
+    */
+
+    /* Do not do newline translation on input. */
+    /*
+    tty_settings.c_iflag &= ~(INLCR | IGNCR | ICRNL);
+    */
+
+#ifdef IUCLC
+    /* Do not do uppercase-to-lowercase mapping on input. */
+    tty_settings.c_iflag &= ~IUCLC;
+#endif
+
+    /* Use 8-bit characters. This too may affect standard streams,
+     * but any sane C library can deal with 8-bit characters. */
+    tty_settings.c_cflag &= ~CSIZE;
+    tty_settings.c_cflag |=  CS8;
+    tty_settings.c_cflag |=  IUTF8;
+
+    /* Enable receiver. */
+    /*
+    tty_settings.c_cflag |=  CREAD;
+    */
+
+    /* Let INTR/QUIT/SUSP/DSUSP generate the corresponding signals. */
+    tty_settings.c_lflag |=  ISIG;
+
+    /* Enable noncanonical mode.
+     * This is the most important bit, as it disables line buffering etc. */
+    /*
+    tty_settings.c_lflag &= ~ICANON;
+    */
+
+    /* Enable echoing input characters. */
+    /*
+    tty_settings.c_lflag |= (ECHO | ECHOE | ECHOK | ECHONL);
+    */
+
+    /* Disable echoing input characters. */
+    /*
+    tty_settings.c_lflag &= ~(ECHO | ECHOE | ECHOK | ECHONL);
+    */
+
+    /* Disable implementation-defined input processing. */
+    /*
+    tty_settings.c_lflag &= ~IEXTEN;
+    */
+
+    /* To maintain best compatibility with normal behaviour of terminals,
+     * we set TIME=0 and MAX=1 in noncanonical mode. This means that
+     * read() will block until at least one byte is available. */
+    /*
+    tty_settings.c_cc[VTIME] = 0;
+    tty_settings.c_cc[VMIN] = 1;
+    */
+
+    /* Set the new terminal settings.
+     * Note that we don't actually check which ones were successfully
+     * set and which not, because there isn't much we can do about it. */
+    tcsetattr(tty_descriptor, TCSANOW, &tty_settings);
+
+    /* Done. */
+    return errno = 0;
+}
+
 /* tty_getty():
  *	Wrapper for tcgetattr to handle EINTR
  */
diff --git a/src/tty.h b/src/tty.h
index 2603e1ad2d..331a507a26 100644
--- a/src/tty.h
+++ b/src/tty.h
@@ -456,6 +456,7 @@ typedef struct {

 typedef unsigned char ttychar_t[NN_IO][C_NCC];

+libedit_private int	tty_save();
 libedit_private int	tty_init(EditLine *);
 libedit_private void	tty_end(EditLine *);
 libedit_private int	tty_stty(EditLine *, int, const wchar_t **);



Home | Main Index | Thread Index | Old Index