[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 <eggert@cs.ucla.edu>
* 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);
}
- Re: CVS problem with ssh, (continued)
- Re: CVS problem with ssh, Derek Price, 2005/07/13
- Re: CVS problem with ssh, Paul Eggert, 2005/07/13
- Re: CVS problem with ssh, Richard M. Stallman, 2005/07/14
- Re: CVS problem with ssh, Paul Eggert, 2005/07/14
- Re: CVS problem with ssh, Richard M. Stallman, 2005/07/15
- Re: CVS problem with ssh, Larry Jones, 2005/07/15
- Re: CVS problem with ssh, Richard M. Stallman, 2005/07/15
- Re: CVS problem with ssh, Larry Jones, 2005/07/15
- Re: CVS problem with ssh, Richard M. Stallman, 2005/07/16
- Re: CVS problem with ssh, Derek Price, 2005/07/15
- Message not available
- Message not available
- Re: CVS problem with ssh,
Paul Eggert <=
- Re: CVS problem with ssh, Alexander Taler, 2005/07/19
Re: CVS problem with ssh, Richard M. Stallman, 2005/07/13
Re: CVS problem with ssh, Paul Eggert, 2005/07/16