Centralise calls to fcntl into functions that carefully check the
authorsimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Fri, 19 Jul 2013 18:10:02 +0000 (18:10 +0000)
committersimon <simon@cda61777-01e9-0310-a592-d414129be87e>
Fri, 19 Jul 2013 18:10:02 +0000 (18:10 +0000)
error returns.

git-svn-id: svn://svn.tartarus.org/sgt/putty@9940 cda61777-01e9-0310-a592-d414129be87e

unix/gtkwin.c
unix/unix.h
unix/uxmisc.c
unix/uxnet.c
unix/uxplink.c
unix/uxproxy.c
unix/uxpty.c

index ee91a93..ad8ade5 100644 (file)
@@ -3307,7 +3307,7 @@ void dup_session_menuitem(GtkMenuItem *item, gpointer gdata)
     }
 
     sprintf(option, "---[%d,%d]", pipefd[0], size);
-    fcntl(pipefd[0], F_SETFD, 0);
+    noncloexec(pipefd[0]);
     fork_and_exec_self(inst, pipefd[1], option, NULL);
     close(pipefd[0]);
 
index 3af9eeb..a9f00bf 100644 (file)
@@ -156,7 +156,10 @@ void (*putty_signal(int sig, void (*func)(int)))(int);
 void block_signal(int sig, int block_it);
 
 /* uxmisc.c */
-int cloexec(int);
+void cloexec(int);
+void noncloexec(int);
+int nonblock(int);
+int no_nonblock(int);
 
 /*
  * Exports from unicode.c.
index 300fc54..473aa94 100644 (file)
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <assert.h>
+#include <errno.h>
 #include <unistd.h>
 #include <time.h>
 #include <sys/time.h>
@@ -168,14 +169,70 @@ void pgp_fingerprints(void)
 }
 
 /*
- * Set FD_CLOEXEC on a file descriptor
+ * Set and clear fcntl options on a file descriptor. We don't
+ * realistically expect any of these operations to fail (the most
+ * plausible error condition is EBADF, but we always believe ourselves
+ * to be passing a valid fd so even that's an assertion-fail sort of
+ * response), so we don't make any effort to return sensible error
+ * codes to the caller - we just log to standard error and die
+ * unceremoniously. However, nonblock and no_nonblock do return the
+ * previous state of O_NONBLOCK.
  */
-int cloexec(int fd) {
+void cloexec(int fd) {
     int fdflags;
 
     fdflags = fcntl(fd, F_GETFD);
-    if (fdflags == -1) return -1;
-    return fcntl(fd, F_SETFD, fdflags | FD_CLOEXEC);
+    if (fdflags < 0) {
+        fprintf(stderr, "%d: fcntl(F_GETFD): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+    if (fcntl(fd, F_SETFD, fdflags | FD_CLOEXEC) < 0) {
+        fprintf(stderr, "%d: fcntl(F_SETFD): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+}
+void noncloexec(int fd) {
+    int fdflags;
+
+    fdflags = fcntl(fd, F_GETFD);
+    if (fdflags < 0) {
+        fprintf(stderr, "%d: fcntl(F_GETFD): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+    if (fcntl(fd, F_SETFD, fdflags & ~FD_CLOEXEC) < 0) {
+        fprintf(stderr, "%d: fcntl(F_SETFD): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+}
+int nonblock(int fd) {
+    int fdflags;
+
+    fdflags = fcntl(fd, F_GETFL);
+    if (fdflags < 0) {
+        fprintf(stderr, "%d: fcntl(F_GETFL): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+    if (fcntl(fd, F_SETFL, fdflags | O_NONBLOCK) < 0) {
+        fprintf(stderr, "%d: fcntl(F_SETFL): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+
+    return fdflags & O_NONBLOCK;
+}
+int no_nonblock(int fd) {
+    int fdflags;
+
+    fdflags = fcntl(fd, F_GETFL);
+    if (fdflags < 0) {
+        fprintf(stderr, "%d: fcntl(F_GETFL): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+    if (fcntl(fd, F_SETFL, fdflags & ~O_NONBLOCK) < 0) {
+        fprintf(stderr, "%d: fcntl(F_SETFL): %s\n", fd, strerror(errno));
+        exit(1);
+    }
+
+    return fdflags & O_NONBLOCK;
 }
 
 FILE *f_open(const Filename *filename, char const *mode, int is_private)
index e951524..02894bd 100644 (file)
@@ -540,7 +540,7 @@ static int try_connect(Actual_Socket sock)
     const union sockaddr_union *sa;
     int err = 0;
     short localport;
-    int fl, salen, family;
+    int salen, family;
 
     /*
      * Remove the socket from the tree before we overwrite its
@@ -695,9 +695,7 @@ static int try_connect(Actual_Socket sock)
        exit(1); /* XXX: GCC doesn't understand assert() on some systems. */
     }
 
-    fl = fcntl(s, F_GETFL);
-    if (fl != -1)
-       fcntl(s, F_SETFL, fl | O_NONBLOCK);
+    nonblock(s);
 
     if ((connect(s, &(sa->sa), salen)) < 0) {
        if ( errno != EINPROGRESS ) {
@@ -1255,7 +1253,6 @@ static int net_select_result(int fd, int event)
            union sockaddr_union su;
            socklen_t addrlen = sizeof(su);
            int t;  /* socket of connection */
-            int fl;
 
            memset(&su, 0, addrlen);
            t = accept(s->s, &su.sa, &addrlen);
@@ -1263,9 +1260,7 @@ static int net_select_result(int fd, int event)
                break;
            }
 
-            fl = fcntl(t, F_GETFL);
-            if (fl != -1)
-                fcntl(t, F_SETFL, fl | O_NONBLOCK);
+            nonblock(t);
 
            if (s->localhost_only &&
                !sockaddr_is_loopback(&su.sa)) {
index 9e10c48..898f27c 100644 (file)
@@ -396,20 +396,18 @@ int try_output(int is_stderr)
     bufchain *chain = (is_stderr ? &stderr_data : &stdout_data);
     int fd = (is_stderr ? STDERR_FILENO : STDOUT_FILENO);
     void *senddata;
-    int sendlen, ret, fl;
+    int sendlen, ret;
 
     if (bufchain_size(chain) > 0) {
-        fl = fcntl(fd, F_GETFL);
-        if (fl != -1 && !(fl & O_NONBLOCK))
-            fcntl(fd, F_SETFL, fl | O_NONBLOCK);
+        int prev_nonblock = nonblock(fd);
         do {
             bufchain_prefix(chain, &senddata, &sendlen);
             ret = write(fd, senddata, sendlen);
             if (ret > 0)
                 bufchain_consume(chain, ret);
         } while (ret == sendlen && bufchain_size(chain) != 0);
-        if (fl != -1 && !(fl & O_NONBLOCK))
-            fcntl(fd, F_SETFL, fl);
+        if (!prev_nonblock)
+            no_nonblock(fd);
         if (ret < 0 && errno != EAGAIN) {
             perror(is_stderr ? "stderr: write" : "stdout: write");
             exit(1);
index 9dee690..aa1ff07 100644 (file)
@@ -306,8 +306,8 @@ Socket platform_new_connection(SockAddr addr, char *hostname,
        dup2(from_cmd_pipe[1], 1);
        close(to_cmd_pipe[0]);
        close(from_cmd_pipe[1]);
-       fcntl(0, F_SETFD, 0);
-       fcntl(1, F_SETFD, 0);
+       noncloexec(0);
+       noncloexec(1);
        execl("/bin/sh", "sh", "-c", cmd, (void *)NULL);
        _exit(255);
     }
index 4a606ef..fb1bd55 100644 (file)
@@ -373,15 +373,7 @@ static void pty_open_master(Pty pty)
     strncpy(pty->name, ptsname(pty->master_fd), FILENAME_MAX-1);
 #endif
 
-    {
-        /*
-         * Set the pty master into non-blocking mode.
-         */
-        int fl;
-       fl = fcntl(pty->master_fd, F_GETFL);
-       if (fl != -1 && !(fl & O_NONBLOCK))
-           fcntl(pty->master_fd, F_SETFL, fl | O_NONBLOCK);
-    }
+    nonblock(pty->master_fd);
 
     if (!ptys_by_fd)
        ptys_by_fd = newtree234(pty_compare_by_fd);