[vte] pty: Cleanup session creation



commit 0c00665c211c6c8cdc4baed27658595b883bed12
Author: Christian Persch <chpe src gnome org>
Date:   Sat Oct 12 20:10:55 2019 +0200

    pty: Cleanup session creation
    
    Remove setpgid() call, since it always fails with EPERM, and doesn't make
    sense after setsid() anyway.
    
    setsid() creates a new session, and loses the controlling TTY.
    
    Furthermore, setsid() is in POSIX, so we can depend on its presence; remove
    the conditional around it.
    
    Fixes: https://gitlab.gnome.org/GNOME/vte/issues/179

 meson.build |  2 --
 src/pty.cc  | 20 +++++++++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)
---
diff --git a/meson.build b/meson.build
index 7858744b..7843cd44 100644
--- a/meson.build
+++ b/meson.build
@@ -186,8 +186,6 @@ check_functions = [
   # Misc PTY handling functions
   'cfmakeraw',
   'getpgid',
-  'setpgid',
-  'setsid',
   'tcsetattr',
   # Misc I/O routines.
   'explicit_bzero',
diff --git a/src/pty.cc b/src/pty.cc
index 372c1cf6..0375f094 100644
--- a/src/pty.cc
+++ b/src/pty.cc
@@ -131,24 +131,22 @@ Pty::child_setup() const noexcept
                 _exit(127);
         }
 
-#if defined(HAVE_SETSID) && defined(HAVE_SETPGID)
         if (!(m_flags & VTE_PTY_NO_SESSION)) {
-                /* Start a new session and become process-group leader.
-                 * This also loses the controlling TTY.
+                /* This starts a new session; we become its process-group leader,
+                 * and lose our controlling TTY.
                  */
                 _vte_debug_print (VTE_DEBUG_PTY, "Starting new session\n");
                 if (setsid() == -1) {
                         _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "setsid");
                         _exit(127);
                 }
-
-                if (setpgid(0, 0) == -1) {
-                        _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "setpgid");
-                        /* _exit(127); */
-                }
         }
-#endif
+        /* FIXME? else if (m_flags & VTE_PTY_NO_CTTTY)
+         * No session and no controlling TTY wanted, do we need to lose our controlling TTY,
+         * perhaps by open("/dev/tty") + ioctl(TIOCNOTTY) ?
+         */
 
+        /* Now open the PTY peer. Note that this also makes the PTY our controlling TTY. */
         /* Note: *not* O_CLOEXEC! */
         auto const fd_flags = int{O_RDWR | ((m_flags & VTE_PTY_NO_CTTY) ? O_NOCTTY : 0)};
         auto fd = int{-1};
@@ -191,6 +189,10 @@ Pty::child_setup() const noexcept
         assert(fd != -1);
 
 #ifdef TIOCSCTTY
+        /* On linux, opening the PTY peer above already made it our controlling TTY (since
+         * previously there was none, after the setsid() call). However, it appears that e.g.
+         * on *BSD, that doesn't happen, so we need this explicit ioctl here.
+         */
         if (!(m_flags & VTE_PTY_NO_CTTY)) {
                 if (ioctl(fd, TIOCSCTTY, fd) != 0) {
                         _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "ioctl(TIOCSCTTY)");


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]