[vte] pty: Don't unset O_NONBLOCK and then set it again

[Christian Persch <chpe src gnome org>]

commit 4d4fd1ed25816781e4f72ddd3f78eb0ca9073bf2
Author: Christian Persch <chpe gnome org>
Date:   Sun Nov 29 20:57:55 2015 +0100

    pty: Don't unset O_NONBLOCK and then set it again
    
    We were unsetting O_NONBLOCK on the PTY master's FD and then
    setting it again. Instead, just always set the flag. Fixes a FIXME.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=747046

 src/pty.cc |   59 ++++++++++++++++++++++++++++++-----------------------------
 src/vte.cc |   11 ++++-------
 2 files changed, 34 insertions(+), 36 deletions(-)
---
diff --git a/src/pty.cc b/src/pty.cc
index cbdebba..2136dbd 100644
--- a/src/pty.cc
+++ b/src/pty.cc
@@ -617,24 +617,28 @@ fd_set_cloexec(int fd)
         return fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
 }
 
-#if defined(HAVE_UNIX98_PTY)
-
-static int
-fd_set_nonblock(int fd,
-                bool set)
+static bool
+fd_set_nonblocking(int fd,
+                   GError **error)
 {
         int flags = fcntl(fd, F_GETFL, 0);
-        if (flags < 0)
-                return flags;
+        if (flags >= 0 &&
+            (flags & O_NONBLOCK) == 0)
+                flags = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
 
-        if (set)
-                flags |= O_NONBLOCK;
-        else
-                flags &= ~(O_NONBLOCK);
-
-        return fcntl(fd, F_SETFL, flags);
+        if (flags < 0) {
+                int errsv = errno;
+                g_set_error(error, VTE_PTY_ERROR,
+                            VTE_PTY_ERROR_PTY98_FAILED,
+                            "%s failed: %s", "Setting O_NONBLOCK flag", g_strerror(errsv));
+                errno = errsv;
+                return FALSE;
+        }
+        return TRUE;
 }
 
+#if defined(HAVE_UNIX98_PTY)
+
 /*
  * _vte_pty_open_unix98:
  * @pty: a #VtePty
@@ -651,7 +655,7 @@ _vte_pty_open_unix98(VtePty *pty,
         VtePtyPrivate *priv = pty->priv;
 
        /* Attempt to open the master. */
-       int fd = posix_openpt(O_RDWR | O_NOCTTY | O_CLOEXEC);
+       int fd = posix_openpt(O_RDWR | O_NOCTTY | O_NONBLOCK | O_CLOEXEC);
         if (fd == -1 && errno == EINVAL) {
                 /* Try without CLOEXEC and apply the flag afterwards */
                 fd = posix_openpt(O_RDWR | O_NOCTTY);
@@ -674,16 +678,6 @@ _vte_pty_open_unix98(VtePty *pty,
                 return -1;
         }
 
-        if (fd_set_nonblock(fd, false) != 0) {
-                int errsv = errno;
-                g_set_error(error, VTE_PTY_ERROR,
-                            VTE_PTY_ERROR_PTY98_FAILED,
-                            "%s failed: %s", "Unsetting O_NONBLOCK flag", g_strerror(errsv));
-                close(fd);
-                errno = errsv;
-                return -1;
-        }
-
         /* tty_ioctl(4) -> every read() gives an extra byte at the beginning
          * notifying us of stop/start (^S/^Q) events. */
         int one = 1;
@@ -763,6 +757,13 @@ _vte_pty_open_bsd(VtePty *pty,
         }
 #endif
 
+        if (!fd_set_nonblocking(parentfd, error)) {
+                int errsv = errno;
+                close(parentfd);
+                errno = errsv;
+                return FALSE;
+        }
+
         /* tty_ioctl(4) -> every read() gives an extra byte at the beginning
          * notifying us of stop/start (^S/^Q) events. */
         int one = 1;
@@ -881,16 +882,16 @@ vte_pty_initable_init (GInitable *initable,
         /* If we already have a (foreign) FD, we're done. */
         if (priv->foreign) {
                 g_assert(priv->pty_fd != -1);
-                return TRUE;
-        }
-
+                ret = fd_set_nonblocking(priv->pty_fd, error);
+        } else {
 #if defined(HAVE_UNIX98_PTY)
-        ret = _vte_pty_open_unix98(pty, error);
+                ret = _vte_pty_open_unix98(pty, error);
 #elif defined(HAVE_OPENPTY)
-        ret = _vte_pty_open_bsd(pty, error);
+                ret = _vte_pty_open_bsd(pty, error);
 #else
 #error Have neither UNIX98 PTY nor BSD openpty!
 #endif
+        }
 
        _vte_debug_print(VTE_DEBUG_PTY,
                        "vte_pty_initable_init returning %s with ptyfd = %d\n",
diff --git a/src/vte.cc b/src/vte.cc
index eb6702e..6605d05 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -10466,16 +10466,13 @@ VteTerminalPrivate::set_pty(VtePty *new_pty)
         m_pty = (VtePty *)g_object_ref(new_pty);
         int pty_master = vte_pty_get_fd(m_pty);
 
+        /* Ensure the FD is non-blocking */
+        int flags = fcntl(pty_master, F_GETFL);
+        g_warn_if_fail(flags >= 0 && (flags & O_NONBLOCK) == O_NONBLOCK);
+
         m_pty_channel = g_io_channel_unix_new(pty_master);
         g_io_channel_set_close_on_unref(m_pty_channel, FALSE);
 
-        /* FIXMEchpe: vte_pty_open_unix98 does the inverse ... */
-        /* Set the pty to be non-blocking. */
-        long flags = fcntl(pty_master, F_GETFL);
-        if ((flags & O_NONBLOCK) == 0) {
-                fcntl(pty_master, F_SETFL, flags | O_NONBLOCK);
-        }
-
         vte_terminal_set_size(m_terminal,
                               m_column_count,
                               m_row_count);