What's wrong with this C code? The child is not returning?

Question

Here's my attempt to try to understand how to do correct piping between two child processes. I'm simply trying to pass the output of one Linux command to another (ls to cat) and have the program return successfully. However, I'm guessing that the second child that is forked is getting stuck and the parent is forever waiting on this child. I have been fiddling with this code for a long time trying to find out why it's getting stuck. I'm kind of a noob when it comes to C system programming, but I am trying to learn.

Does anybody have any idea why the program does not exit, but hangs on cat?

Any help would be greatly appreciated.

Thanks.

#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/stat.h>
#include <fcntl.h>
int main()
{
    char *a[2] = {"/bin/ls", NULL};
    char *b[2] = {"/bin/cat", NULL};
    char *envp[2] = {getenv("PATH"), NULL};
    int fd[2], status;
    pipe(fd);
    int old_std_out = dup(1);
    int old_std_in = dup(0);
    dup2(fd[1], 1);
    int pid = fork();
    switch(pid)
    {
        case -1:
            perror("Forkscrew");
            exit(1);
            break;
        case 0:
            execve(a[0], a, envp);
            exit(0);
            break;
       default:
            waitpid(-1, &status, 0);
            dup2(old_std_out, 1);
            break;
    }
    dup2(fd[0], 0);
    pid = fork();
    switch(pid)
    {
        case -1:
            perror("Forkscrew");
            exit(1);
            break;
        case 0:
            execve(b[0], b, envp);
            exit(0);
            break;
        default:
            waitpid(-1, &status, 0);
            dup2(old_std_in, 0);
            break;
    }
    printf("\n");
    return 0;
}

Answer 1

There are two potential deadlocks in your program.

First, the first child (ls) may block when trying to write to the pipe, in which case waitpid() will not return until ls terminates, and ls will not terminate until the second child (cat) starts executing, which can't happen until waitpid() returns. => Deadlock.

Second, cat will read from its stdin until all file descriptors for the write end are closed. Both the parent process and cat have a copy of the write end, cat has it without knowing about it explicitly. Some operating systems will have read() not block if the only copy of the write end is in the same process (to avoid this deadlock), but this is not guaranteed. Either way, since the parent process keeps around a copy of the filedescriptor, and the parent process waitpid()s for the child, which waits for the write-end of the pipe to close, you have deadlock again.

Often, simplifying the program solves such problems:

#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/stat.h>
#include <fcntl.h>
int main()
{
    char *a[2] = {"/bin/ls", NULL};
    char *b[2] = {"/bin/cat", NULL};
    char *envp[2] = {getenv("PATH"), NULL};
    int fd[2], status;
    pipe(fd);
    //int old_std_out = dup(1); /*No need to copy stdout...*/
    //int old_std_in = dup(0);  /*...or stdin...*/
    //dup2(fd[1], 1);           /*...if you wait dup2()ing until you need to*/
    int pid = fork();
    switch(pid)
    {
        case -1:
            perror("Forkscrew");
            exit(1);
            //break; /*unreachable*/
        case 0:
            dup2(fd[1], STDOUT_FILENO); /*NOW we dup2()*/
            close(fd[0]); /*no need to pass these file descriptors to...*/
            close(fd[1]); /*...a program that doesn't expect to have them open*/ 
            execve(a[0], a, envp);
            exit(0); /*might want an error message*/
            //break; /*unreachable*/
       default:
            //waitpid(-1, &status, 0); /*don't wait yet*/
            //dup2(old_std_out, 1);
            close(fd[1]); /*we don't need this in the parent anymore*/
            break;
    }
    //dup2(fd[0], 0); /*not needed anymore*/
    pid = fork();
    switch(pid)
    {
        case -1:
            perror("Forkscrew");
            /*might want to ensure the first child can terminate*/
            exit(1);
            //break; /*unreachable*/
        case 0:
            dup2(fd[0], STDIN_FILENO);
            close(fd[0]); /*again, cat doesn't expect a fourth fd open*/
            execve(b[0], b, envp);
            /*again, error message would be nice*/
            exit(0);
            //break;
        default:
            //waitpid(-1, &status, 0);
            //dup2(old_std_in, 0);
            break;
    }
    waitpid(-1, &status, 0); /*don't wait until both children are created*/
    waitpid(-1, &status, 0);
    printf("\n");
    return 0;
}

As you see, I've left a number of suggestions for improvements, but this should already work fine now, provided the execve()s work alright.