bug-cvs
[Top][All Lists]
Advanced

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

Re: CVS problem with ssh


From: Paul Eggert
Subject: Re: CVS problem with ssh
Date: Sat, 16 Jul 2005 18:13:03 -0700
User-agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.4 (gnu/linux)

OK, here's a proposed patch to CVS 1.12.12 for the CVS + ssh problem.
It uses a pipe, but leaves stdio alone.

Unfortunately I can't reproduce the bug with CVS 1.12.12, so I can't
really test that the patch fixes the bug in general.  Can someone else
reproduce it?

2005-07-16  Paul Eggert  <address@hidden>

        * src/client.c (handle_m, handle_e): Remove workaround for O_NONBLOCK
        problem; no longer needed because of the fix below.
        * src/cvs.h (piped_child): New bool argument saying whether O_NONBLOCK
        fix is needed.  All uses changed.
        * src/rsh-client.c (start_rsh_server): We need the O_NONBLOCK fix,
        so pass 'true' to piped_child to enable the workaround.
        * src/run.c (work_around_openssh_glitch): New function.
        (piped_child): Use it if the fix is requested.

diff -pubr cvs-1.12.12/src/client.c cvs-1.12.12-nonblock/src/client.c
--- cvs-1.12.12/src/client.c    2005-04-14 07:13:25.000000000 -0700
+++ cvs-1.12.12-nonblock/src/client.c   2005-07-15 10:49:52.000000000 -0700
@@ -2718,9 +2718,6 @@ handle_wrapper_rcs_option (char *args, s
 static void
 handle_m (char *args, size_t len)
 {
-    fd_set wfds;
-    int s;
-
     /* In the case where stdout and stderr point to the same place,
        fflushing stderr will make output happen in the correct order.
        Often stderr will be line-buffered and this won't be needed,
@@ -2728,12 +2725,6 @@ handle_m (char *args, size_t len)
        based on being confused between default buffering between
        stdout and stderr.  But I'm not sure).  */
     fflush (stderr);
-    FD_ZERO (&wfds);
-    FD_SET (STDOUT_FILENO, &wfds);
-    errno = 0;
-    s = select (STDOUT_FILENO+1, NULL, &wfds, NULL, NULL);
-    if (s < 1 && errno != 0)
-        perror ("cannot write to stdout");
     fwrite (args, len, sizeof (*args), stdout);
     putc ('\n', stdout);
 }
@@ -2779,24 +2770,9 @@ handle_mbinary (char *args, size_t len)
 static void
 handle_e (char *args, size_t len)
 {
-    fd_set wfds;
-    int s;
-
     /* In the case where stdout and stderr point to the same place,
        fflushing stdout will make output happen in the correct order.  */
     fflush (stdout);
-    FD_ZERO (&wfds);
-    FD_SET (STDERR_FILENO, &wfds);
-    errno = 0;
-    s = select (STDERR_FILENO+1, NULL, &wfds, NULL, NULL);
-    /*
-     * If stderr has problems, then adding a call to
-     *   perror ("cannot write to stderr")
-     * will not work. So, try to write a message on stdout and
-     * terminate cvs.
-     */
-    if (s < 1 && errno != 0)
-        fperrmsg (stdout, 1, errno, "cannot write to stderr");
     fwrite (args, len, sizeof (*args), stderr);
     putc ('\n', stderr);
 }
@@ -3758,7 +3734,7 @@ connect_to_forked_server (cvsroot_t *roo
     TRACE (TRACE_FUNCTION, "Forking server: %s %s",
           command[0] ? command[0] : "(null)", command[1]);
 
-    child_pid = piped_child (command, &tofd, &fromfd);
+    child_pid = piped_child (command, &tofd, &fromfd, false);
     if (child_pid < 0)
        error (1, 0, "could not fork server process");
 
diff -pubr cvs-1.12.12/src/cvs.h cvs-1.12.12-nonblock/src/cvs.h
--- cvs-1.12.12/src/cvs.h       2005-04-14 07:14:55.000000000 -0700
+++ cvs-1.12.12-nonblock/src/cvs.h      2005-07-15 10:20:08.000000000 -0700
@@ -666,7 +666,7 @@ int run_piped (int *, int *);
 
 /* other similar-minded stuff from run.c.  */
 FILE *run_popen (const char *, const char *);
-int piped_child (char *const *, int *, int *);
+int piped_child (char *const *, int *, int *, bool);
 void close_on_exec (int);
 
 pid_t waitpid (pid_t, int *, int);
diff -pubr cvs-1.12.12/src/rsh-client.c cvs-1.12.12-nonblock/src/rsh-client.c
--- cvs-1.12.12/src/rsh-client.c        2005-03-15 09:45:10.000000000 -0800
+++ cvs-1.12.12-nonblock/src/rsh-client.c       2005-07-15 10:20:01.000000000 
-0700
@@ -184,7 +184,7 @@ start_rsh_server (cvsroot_t *root, struc
                fprintf (stderr, "%s ", argv[i]);
            putc ('\n', stderr);
        }
-       child_pid = piped_child (argv, &tofd, &fromfd);
+       child_pid = piped_child (argv, &tofd, &fromfd, true);
 
        if (child_pid < 0)
            error (1, errno, "cannot start server via rsh");
diff -pubr cvs-1.12.12/src/run.c cvs-1.12.12-nonblock/src/run.c
--- cvs-1.12.12/src/run.c       2005-04-14 07:13:26.000000000 -0700
+++ cvs-1.12.12-nonblock/src/run.c      2005-07-15 20:35:14.000000000 -0700
@@ -430,9 +430,100 @@ run_popen (const char *cmd, const char *
 }
 
 
+/* Work around an OpenSSH problem: it can put its standard file
+   descriptors into nonblocking mode, which will mess us up if we
+   share file descriptions with it.  The simplest workaround is
+   to create an intervening process between OpenSSH and the
+   actual stderr.  */
+
+static void
+work_around_openssh_glitch (void)
+{
+    pid_t pid;
+    int stderr_pipe[2];
+    struct stat sb;
+
+    /* Do nothing unless stderr is a file that is affected by
+       nonblocking mode.  */
+    if (! (fstat (STDERR_FILENO, &sb) == 0
+          && (S_ISFIFO (sb.st_mode) || S_ISSOCK (sb.st_mode)
+              || S_ISCHR (sb.st_mode) || S_ISBLK (sb.st_mode))))
+       return;
+
+    if (pipe (stderr_pipe) < 0)
+       error (1, errno, "cannot create pipe");
+    pid = fork ();
+    if (pid < 0)
+       error (1, errno, "cannot fork");
+    if (pid != 0)
+    {
+       /* Still in child of original process.  Act like "cat -u".  */
+       char buf[1 << 13];
+       ssize_t inbytes;
+       pid_t w;
+       int status;
+
+       if (close (stderr_pipe[1]) < 0)
+           error (1, errno, "cannot close pipe");
+
+       while ((inbytes = read (stderr_pipe[0], buf, sizeof buf)) != 0)
+       {
+           size_t outbytes = 0;
+
+           if (inbytes < 0)
+           {
+               if (errno == EINTR)
+                   continue;
+               error (1, errno, "reading from pipe");
+           }
+
+           do
+           {
+               ssize_t w = write (STDERR_FILENO,
+                                  buf + outbytes, inbytes - outbytes);
+               if (w < 0)
+               {
+                   if (errno = EINTR)
+                     w = 0;
+                   if (w < 0)
+                     _exit (1);
+               }
+               outbytes += w;
+           }
+           while (inbytes != outbytes);
+       }
+
+       /* Done processing output from grandchild.  Propagate
+          its exit status back to the parent.  */
+       while ((w = waitpid (pid, &status, 0)) == -1 && errno == EINTR)
+           continue;
+       if (w < 0)
+           error (1, errno, "waiting for child");
+       if (! WIFEXITED (status))
+       {
+           if (WIFSIGNALED (status))
+               raise (WTERMSIG (status));
+           error (1, errno, "child did not exit cleanly");
+       }
+       _exit (WEXITSTATUS (status));
+    }
+
+    /* Grandchild of original process.  */
+    if (close (stderr_pipe[0]) < 0)
+       error (1, errno, "cannot close pipe");
+
+    if (stderr_pipe[1] != STDERR_FILENO)
+    {
+       if (dup2 (stderr_pipe[1], STDERR_FILENO) < 0)
+           error (1, errno, "cannot dup2 pipe");
+       if (close (stderr_pipe[1]) < 0)
+           error (1, errno, "cannot close pipe");
+    }
+}
+
 
 int
-piped_child (char *const *command, int *tofdp, int *fromfdp)
+piped_child (char *const *command, int *tofdp, int *fromfdp, bool fix_stderr)
 {
     int pid;
     int to_child_pipe[2];
@@ -468,6 +559,9 @@ piped_child (char *const *command, int *
        if (dup2 (from_child_pipe[1], STDOUT_FILENO) < 0)
            error (1, errno, "cannot dup2 pipe");
 
+       if (fix_stderr)
+         work_around_openssh_glitch ();
+
        /* Okay to cast out const below - execvp don't return anyhow.  */
        execvp ((char *)command[0], (char **)command);
        error (1, errno, "cannot exec %s", command[0]);
@@ -488,7 +582,7 @@ int
 run_piped (int *tofdp, int *fromfdp)
 {
     run_add_arg (NULL);
-    return piped_child (run_argv, tofdp, fromfdp);
+    return piped_child (run_argv, tofdp, fromfdp, false);
 }
 
 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]