[vte] pty: Split getting PTY peer FD out of child setup

[Christian Persch <chpe src gnome org>]

commit c9dc0863888e536f002059550e205cdc1c03d701
Author: Christian Persch <chpe src gnome org>
Date:   Mon Apr 27 20:49:04 2020 +0200

    pty: Split getting PTY peer FD out of child setup
    
    It will be needed separately when spawning via the portal.

 src/pty.cc | 168 +++++++++++++++++++++++++++++++++----------------------------
 src/pty.hh |   2 +
 2 files changed, 92 insertions(+), 78 deletions(-)
---
diff --git a/src/pty.cc b/src/pty.cc
index 30447215..040a0005 100644
--- a/src/pty.cc
+++ b/src/pty.cc
@@ -104,50 +104,22 @@ Pty::unref() noexcept
                 delete this;
 }
 
-void
-Pty::child_setup() const noexcept
+vte::libc::FD
+Pty::get_peer() const noexcept
 {
-        /* Unblock all signals */
-        sigset_t set;
-        sigemptyset(&set);
-        if (pthread_sigmask(SIG_SETMASK, &set, nullptr) == -1) {
-                _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "pthread_sigmask");
-                _exit(127);
-        }
-
-        /* Reset the handlers for all signals to their defaults.  The parent
-         * (or one of the libraries it links to) may have changed one to be ignored. */
-        for (int n = 1; n < NSIG; n++) {
-                if (n == SIGSTOP || n == SIGKILL)
-                        continue;
-
-                signal(n, SIG_DFL);
-        }
-
-        auto masterfd = fd();
-        if (masterfd == -1)
-                _exit(127);
+        if (!m_pty_fd)
+                return {};
 
-        if (grantpt(masterfd) != 0) {
+        if (grantpt(m_pty_fd.get()) != 0) {
                 _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "grantpt");
-                _exit(127);
+                return {};
         }
 
-       if (unlockpt(masterfd) != 0) {
+        if (unlockpt(m_pty_fd.get()) != 0) {
                 _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "unlockpt");
-                _exit(127);
+                return {};
         }
 
-        if (!(m_flags & VTE_PTY_NO_SESSION)) {
-                /* 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);
-                }
-        }
         /* 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) ?
@@ -156,44 +128,100 @@ Pty::child_setup() const noexcept
         /* 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};
+
+        auto peer_fd = vte::libc::FD{};
 
 #ifdef __linux__
-        fd = ioctl(masterfd, TIOCGPTPEER, fd_flags);
+        peer_fd = ioctl(m_pty_fd.get(), TIOCGPTPEER, fd_flags);
         /* Note: According to the kernel's own tests (tools/testing/selftests/filesystems/devpts_pts.c),
          * the error returned when the running kernel does not support this ioctl should be EINVAL.
          * However it appears that the actual error returned is ENOTTY. So we check for both of them.
          * See issue#182.
          */
-        if (fd == -1 &&
+        if (!peer_fd &&
             errno != EINVAL &&
             errno != ENOTTY) {
-               _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "ioctl(TIOCGPTPEER)");
-               _exit(127);
+                _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "ioctl(TIOCGPTPEER)");
+                return {};
         }
 
         /* Fall back to ptsname + open */
 #endif
 
-        if (fd == -1) {
-                auto const name = ptsname(masterfd);
+        if (!peer_fd) {
+                auto const name = ptsname(m_pty_fd.get());
                 if (name == nullptr) {
                         _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "ptsname");
-                        _exit(127);
+                        return {};
                 }
 
                 _vte_debug_print (VTE_DEBUG_PTY,
                                   "Setting up child pty: master FD = %d name = %s\n",
-                                  masterfd, name);
+                                  m_pty_fd.get(), name);
 
-                fd = ::open(name, fd_flags);
-                if (fd == -1) {
+                peer_fd = ::open(name, fd_flags);
+                if (!peer_fd) {
                         _vte_debug_print (VTE_DEBUG_PTY, "Failed to open PTY: %m\n");
+                        return {};
+                }
+        }
+
+        assert(bool(peer_fd));
+
+#if defined(__sun) && defined(HAVE_STROPTS_H)
+        if (isastream (peer_fd.get()) == 1) {
+                if ((ioctl(peer_fd.get(), I_FIND, "ptem") == 0) &&
+                    (ioctl(peer_fd.get(), I_PUSH, "ptem") == -1)) {
+                        return {};
+                }
+                if ((ioctl(peer_fd.get(), I_FIND, "ldterm") == 0) &&
+                    (ioctl(peer_fd.get(), I_PUSH, "ldterm") == -1)) {
+                        return {};
+                }
+                if ((ioctl(peer_fd.get(), I_FIND, "ttcompat") == 0) &&
+                    (ioctl(peer_fd.get(), I_PUSH, "ttcompat") == -1)) {
+                        return {};
+                }
+        }
+#endif
+
+        return peer_fd;
+}
+
+void
+Pty::child_setup() const noexcept
+{
+        /* Unblock all signals */
+        sigset_t set;
+        sigemptyset(&set);
+        if (pthread_sigmask(SIG_SETMASK, &set, nullptr) == -1) {
+                _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "pthread_sigmask");
+                _exit(127);
+        }
+
+        /* Reset the handlers for all signals to their defaults.  The parent
+         * (or one of the libraries it links to) may have changed one to be ignored. */
+        for (int n = 1; n < NSIG; n++) {
+                if (n == SIGSTOP || n == SIGKILL)
+                        continue;
+
+                signal(n, SIG_DFL);
+        }
+
+        if (!(m_flags & VTE_PTY_NO_SESSION)) {
+                /* 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);
                 }
         }
 
-        assert(fd != -1);
+        auto peer_fd = get_peer();
+        if (!peer_fd)
+                _exit(127);
 
 #ifdef TIOCSCTTY
         /* On linux, opening the PTY peer above already made it our controlling TTY (since
@@ -201,53 +229,37 @@ Pty::child_setup() const noexcept
          * 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) {
+                if (ioctl(peer_fd.get(), TIOCSCTTY, peer_fd.get()) != 0) {
                         _vte_debug_print(VTE_DEBUG_PTY, "%s failed: %m\n", "ioctl(TIOCSCTTY)");
                         _exit(127);
                 }
         }
 #endif
 
-#if defined(__sun) && defined(HAVE_STROPTS_H)
-       if (isastream (fd) == 1) {
-               if ((ioctl(fd, I_FIND, "ptem") == 0) &&
-                               (ioctl(fd, I_PUSH, "ptem") == -1)) {
-                       _exit (127);
-               }
-               if ((ioctl(fd, I_FIND, "ldterm") == 0) &&
-                               (ioctl(fd, I_PUSH, "ldterm") == -1)) {
-                       _exit (127);
-               }
-               if ((ioctl(fd, I_FIND, "ttcompat") == 0) &&
-                               (ioctl(fd, I_PUSH, "ttcompat") == -1)) {
-                       perror ("ioctl (fd, I_PUSH, \"ttcompat\")");
-                       _exit (127);
-               }
-       }
-#endif
-
        /* now setup child I/O through the tty */
-       if (fd != STDIN_FILENO) {
-               if (dup2(fd, STDIN_FILENO) != STDIN_FILENO){
+       if (peer_fd != STDIN_FILENO) {
+               if (dup2(peer_fd.get(), STDIN_FILENO) != STDIN_FILENO){
                        _exit (127);
                }
        }
-       if (fd != STDOUT_FILENO) {
-               if (dup2(fd, STDOUT_FILENO) != STDOUT_FILENO){
+       if (peer_fd != STDOUT_FILENO) {
+               if (dup2(peer_fd.get(), STDOUT_FILENO) != STDOUT_FILENO){
                        _exit (127);
                }
        }
-       if (fd != STDERR_FILENO) {
-               if (dup2(fd, STDERR_FILENO) != STDERR_FILENO){
+       if (peer_fd != STDERR_FILENO) {
+               if (dup2(peer_fd.get(), STDERR_FILENO) != STDERR_FILENO){
                        _exit (127);
                }
        }
 
-       /* Close the original FD, unless it's one of the stdio descriptors */
-       if (fd != STDIN_FILENO &&
-                       fd != STDOUT_FILENO &&
-                       fd != STDERR_FILENO) {
-               close(fd);
+       /* If the peer FD has not been consumed above as one of the stdio descriptors,
+         * need to close it now so that it doesn't leak to the child.
+         */
+       if (peer_fd == STDIN_FILENO  ||
+            peer_fd == STDOUT_FILENO ||
+            peer_fd == STDERR_FILENO) {
+                (void)peer_fd.release();
        }
 
         /* Now set the TERM environment variable */
diff --git a/src/pty.hh b/src/pty.hh
index dea19688..ace59f3d 100644
--- a/src/pty.hh
+++ b/src/pty.hh
@@ -40,6 +40,8 @@ private:
 
         VtePtyFlags m_flags{VTE_PTY_DEFAULT};
 
+        vte::libc::FD get_peer() const noexcept;
+
 public:
         constexpr Pty(vte::libc::FD&& fd,
                       VtePtyFlags flags = VTE_PTY_DEFAULT) noexcept