libglusterfs: run: fix closing of redirection fd in child

When run was doing a program invocation like "echo foo > foof",
the file descriptor to foof was closed before stdout could be
reopened to it.

To fix this, we change the order of actions: first do redirections,
then do the fd cleanup. This also simplifies code (as there are less
fds which are exempt of the cleanup).

Change-Id: I5f74e41f2999e59916a40c8a89d7fb730d808c27
BUG: 2562
Reported-by: Rajesh Amaravathi <rajesh@gluster.com>
Reviewed-on: http://review.gluster.com/428
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Tested-by: Rajesh Amaravathi <rajesh@gluster.com>
Reviewed-by: Rajesh Amaravathi <rajesh@gluster.com>
Reviewed-by: Amar Tumballi <amar@gluster.com>

1 files changed, 38 insertions, 30 deletions

diff --git a/libglusterfs/src/run.c b/libglusterfs/src/run.c
index ff23b02ef38..46bb37d7374 100644
--- a/libglusterfs/src/run.c
+++ b/libglusterfs/src/run.c
@@ -257,33 +257,6 @@ runner_start (runner_t *runner)
                         close (pi[i][i ? 0 : 1]);
                 close (xpi[0]);
                 ret = 0;
-#ifdef GF_LINUX_HOST_OS
-                {
-                        DIR *d = NULL;
-                        struct dirent *de = NULL;
-                        char *e = NULL;
-
-                        d = opendir ("/proc/self/fd");
-                        if (d) {
-                                while ((de = readdir (d))) {
-                                        i = strtoul (de->d_name, &e, 10);
-                                        if (*e == '\0' &&
-                                            i > 2 && i != dirfd (d) &&
-                                            i != pi[0][0] && i != pi[1][1] &&
-                                            i != pi[2][1] && i != xpi[1])
-                                                close (i);
-                                }
-                                closedir (d);
-                        } else
-                                ret = -1;
-                }
-#else
-                for (i = 3; i < 65536; i++) {
-                        if (i != pi[0][0] && i != pi[1][1] &&
-                            i != pi[2][1] && i != xpi[1])
-                                close (i);
-                }
-#endif
 
                 for (i = 0; i < 3; i++) {
                         if (ret == -1)
@@ -295,9 +268,6 @@ runner_start (runner_t *runner)
                         case -2:
                                 /* redir to pipe */
                                 ret = dup2 (pi[i][i ? 1 : 0], i);
-                                errno_priv = errno;
-                                close (pi[i][i ? 1 : 0]);
-                                errno = errno_priv;
                                 break;
                         default:
                                 /* redir to file */
@@ -305,6 +275,31 @@ runner_start (runner_t *runner)
                         }
                 }
 
+                if (ret != -1 ) {
+#ifdef GF_LINUX_HOST_OS
+                        DIR *d = NULL;
+                        struct dirent *de = NULL;
+                        char *e = NULL;
+
+                        d = opendir ("/proc/self/fd");
+                        if (d) {
+                                while ((de = readdir (d))) {
+                                        i = strtoul (de->d_name, &e, 10);
+                                        if (*e == '\0' && i > 2 &&
+                                            i != dirfd (d) && i != xpi[1])
+                                                close (i);
+                                }
+                                closedir (d);
+                        } else
+                                ret = -1;
+#else
+                        for (i = 3; i < 65536; i++) {
+                                if (i != xpi[1])
+                                        close (i);
+                        }
+#endif
+                }
+
                 if (ret != -1) {
                         /* save child from inheriting our singal handling */
                         sigemptyset (&set);
@@ -433,6 +428,7 @@ main ()
         char buf[80];
         char *wdbuf;;
         int ret;
+        int fd;
         long pathmax = pathconf ("/", _PC_PATH_MAX);
 
         wdbuf = malloc (pathmax);
@@ -471,6 +467,18 @@ main ()
         ret = runcmd ("bafflavvitty", NULL);
         printf ("%d %d [%s]\n", ret, errno, strerror (errno));
 
+        TBANNER ("output redirection");
+        fd = open ("/tmp/foof", O_WRONLY|O_CREAT|O_TRUNC, 0600);
+        assert (fd != -1);
+        runinit (&runner);
+        runner_add_args (&runner, "echo", "foo", NULL);
+        runner_redir (&runner, 1, fd);
+        ret = runner_run (&runner);
+        printf ("%d", ret);
+        if (ret != 0)
+                printf (" %d [%s]", errno, strerror (errno));
+        putchar ('\n');
+
         return 0;
 }
 #endif