[PATCH] redirect stderr to /dev/null in piped_child()

[Ted Phelps]

Here's my bug and a patch which fixes it for me.  I haven't been
following the mailing list, so I apologize if this has already been
fixed.

Cheers,
-Ted


BUG DESCRIPTION

I'm using pcl-cvs v2.9.9 in xemacs-21.4.5 on Linux (glibc-2.2.4,
kernel-2.4.16) with cvs-1.11.1p1 in client/server mode using
openssh-2.9b2 as the CVS_RSH process.  Enabling compression (-z 3)
appears to exacerbate the following problem, although it still
occasionally appears when compression is disabled.

Often when using the interactive diff (cvs-mode-idiff) function, the
base file (obtained by xemacs with cvs -q update -p -r HEAD <file>)
would be numerous missing 4K chunks, with the first dropout
consistently appearing exactly 4096 bytes from the start.  Running the
cvs command from the command-line always produces correct results.
The difference between the command-line and xemacs's invocation is
that the latter sets stdout and stderr to be the same file descriptor
when spawning cvs as a subprocess.  I cannot reproduce the behavior
with tcsh or bash using redirection, but I have written a short C
program (see below) which does.

I tracked the problem down to cvs passing the file descriptor for
stderr through to the RSH process.  I would expect stderr to be
redirected to /dev/null.

The patch below does this and should work for all UN*X systems.  I'm
not sure what would be appropriate for Windows, Amiga, etc.


LEGALESE

I grant permission to distribute this patch under the terms of the GNU
Public License.


PATCH

--- cvs-1.11.1p1/src/run.c-ORIG Wed Nov 28 17:24:29 2001
+++ cvs-1.11.1p1/src/run.c      Fri Dec  7 19:30:26 2001
@@ -405,6 +405,7 @@
     int pid;
     int to_child_pipe[2];
     int from_child_pipe[2];
+    int error_fd;
 
     if (pipe (to_child_pipe) < 0)
        error (1, errno, "cannot create pipe");
@@ -418,6 +419,9 @@
     setmode (from_child_pipe[1], O_BINARY);
 #endif
 
+    if ((error_fd = open("/dev/null", O_RDONLY)) < 0)
+       error (1, errno, "cannot open /dev/null");
+
 #ifdef HAVE_VFORK
     pid = vfork ();
 #else
@@ -435,6 +439,10 @@
            error (1, errno, "cannot close pipe");
        if (dup2 (from_child_pipe[1], STDOUT_FILENO) < 0)
            error (1, errno, "cannot dup2 pipe");
+       if (dup2 (error_fd, STDERR_FILENO) < 0)
+           error (1, errno, "cannot dup2 pipe");
+       if (close (error_fd) < 0)
+           error (1, errno, "cannot close pipe");
 
        execvp (command[0], command);
        error (1, errno, "cannot exec %s", command[0]);


PROGRAM

Compile:
> gcc -o foo foo.c

Run:
> foo cvs -q update -p -r HEAD filename.c

--- foo.c ---

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
#include <wait.h>

#define BUFFER_SIZE 512


int main(int argc, char *argv[])
{
    int fds[2], ifd, efd;
    pid_t pid;
    ssize_t total_length;

    /* Do that wacky pipe thang */
    if (pipe(fds) < 0) 
    {
        perror("pipe(): failed");
        exit(1);
    }

    /* Stdin comes from /dev/null */
    if ((ifd = open("/dev/null", O_RDONLY)) < 0)
    {
        perror("open(): failed");
        exit(1);
    }

    /* Stderr goes to stdout */
    efd = fds[1];

    /* Fork a child process */
    if ((pid = fork()) == (pid_t)-1)
    {
        perror("fork(): failed");
        exit(1);
    }

    /* Are we the child? */
    if (pid == 0)
    {
        /* Copy the input fd onto stdin */
        if (dup2(ifd, STDIN_FILENO) < 0)
        {
            perror("dup2(): failed");
            exit(1);
        }

        /* Copy the output fd onto stdout */
        if (dup2(fds[1], STDOUT_FILENO) < 0)
        {
            perror("dup2(): failed");
            exit(1);
        }

        /* Copy the error fd onto stderr */
        if (dup2(efd, STDERR_FILENO) < 0)
        {
            perror("dup2(): failed");
            exit(1);
        }

        /* Close the reading descriptor */
        if (close(fds[0]) < 0)
        {
            perror("close(): failed");
            exit(1);
        }

        if (close(fds[1]) < 0)
        {
            perror("close(): failed");
            exit(1);
        }

        /* Execute the other process */
        if (execvp(argv[1], argv + 1) < 0)
        {
            perror("execl(): failed");
            exit(1);
        }

        /* Should never get here */
        abort();
    }

    /* We're the parent process */
    if (close(fds[1]) < 0)
    {
        perror("close(): failed");
        exit(1);
    }

    /* Read from the pipe until out of input */
    total_length = 0;
    while (1)
    {
        char buffer[BUFFER_SIZE];
        ssize_t length;

        if ((length = read(fds[0], buffer, BUFFER_SIZE)) < 0)
        {
            perror("read(): failed");
            exit(1);
        }

        if (length == 0)
        {
            int status;

            if (close(fds[0]) < 0)
            {
                perror("close(): failed");
                exit(1);
            }

            fprintf(stderr, "total length read: %d\n", total_length);

            if (waitpid(pid, &status, 0) < 0)
            {
                perror("waitpid(): failed");
                exit(1);
            }

            fprintf(stderr, "child exited: %d\n", WEXITSTATUS(status));
            exit(0);
        }

        fwrite(buffer, length, 1, stdout);
        total_length += length;
    }
}