Classic C. Using pipes in execvp function, stdin and stdout redirection

Question

I want to simulate bash in my Linux C program using pipes and execvp function. e.g

ls -l | wc -l  

There is my program:

if(pipe(des_p) == -1) {perror("Failed to create pipe");}

if(fork() == 0) {    //first fork
  close(1);          //closing stdout
  dup(des_p[1]);     //replacing stdout with pipe write 
  close(des_p[0]);   //closing pipe read
  close(des_p[1]);   //closing pipe write

  if(execvp(bash_args[0], bash_args)) // contains ls -l
    /* error checking */
}
else {
  if(fork() == 0) {  //creating 2nd child
    close(0);        //closing stdin
    dup(des_p[0]);   //replacing stdin with pipe read
    close(des_p[1]); //closing pipe write
    close(des_p[0]); //closing pipe read

    if(execvp(bash_args[another_place], bash_args)) //contains wc -l
      /* error checking */
  }

  close(des_p[0]);
  close(des_p[1]);
  wait(0);
  wait(0);
}

This code actually runs, but doesn't do the right thing. What's wrong with this code? That's not working and I don't have a clue why.

Solution 1

You need to close the pipe fds in the parent, or the child won't receive EOF, because the pipe's still open for writing in the parent. This would cause the second wait() to hang. Works for me:

#include <unistd.h>
#include <stdlib.h>


int main(int argc, char** argv)
{
        int des_p[2];
        if(pipe(des_p) == -1) {
          perror("Pipe failed");
          exit(1);
        }

        if(fork() == 0)            //first fork
        {
            close(STDOUT_FILENO);  //closing stdout
            dup(des_p[1]);         //replacing stdout with pipe write 
            close(des_p[0]);       //closing pipe read
            close(des_p[1]);

            const char* prog1[] = { "ls", "-l", 0};
            execvp(prog1[0], prog1);
            perror("execvp of ls failed");
            exit(1);
        }

        if(fork() == 0)            //creating 2nd child
        {
            close(STDIN_FILENO);   //closing stdin
            dup(des_p[0]);         //replacing stdin with pipe read
            close(des_p[1]);       //closing pipe write
            close(des_p[0]);

            const char* prog2[] = { "wc", "-l", 0};
            execvp(prog2[0], prog2);
            perror("execvp of wc failed");
            exit(1);
        }

        close(des_p[0]);
        close(des_p[1]);
        wait(0);
        wait(0);
        return 0;
}

Solution 2

Read up on what the wait function does. It will wait until one child process exists. You're waiting for the first child to exit before you start the second child. The first child probably won't exit until there's some process that reads from the other end of the pipe.

Comments

[krzakov]

When I remove first wait(), there's madness on the screen. Execvp starting after main program ends. How to change it then?

[Art]

Create processes with the pipe between then, then wait twice for both of them to finish.

[Nicholas Wilson]

@krzakov You need to close the pipe fds in the parent, or the second child won't receive EOF, and so will hang (unless the first child is very keen and calls shutdown() on its end).

[Nicholas Wilson]

@krzakov Looks familiar... That was my answer, minus the fact that you aren't waiting for both children as Art pointed out you should do. It's correct if it does what you want. All I can vouch for is that my code compiles and does what I think you want! (I undid your changes to the question, since otherwise it's confusing.)

[krzakov]

Man You're boss. Finally works (I edited first code and now works).

[Jonathan Leffler]

There's a pretty reliable Rule of Thumb: If you use dup() or dup2() to duplicate one end of a pipe to standard input or standard output, you need to close() both ends of the original pipe. There could be circumstances where this is not the necessary behaviour, but such circumstances are very seldom encountered.

[Asad-ullah Khan]

Just wanted to point out that if you have multiple execvp calls that communicate with each other via pipes, only close the write end of the pipes in the parent (close both in child). If you close both in parent, subsequent calls to waitpid will hang.

[Nicholas Wilson]

Are you thinking of shutdown, not close? When forking a child, you do want to close every fd that the child will use.

[Asad-ullah Khan]

Take a look at this code at line 61: Dynpipe. Uncommenting that line and then running ./Dynpipe "ls -l" "grep cpp" "wc -l" will result in a hang. I believe this is because the parent's call to close read/write happens too fast, and the pipe pointed to by the file descriptor gets deleted.

[Nicholas Wilson]

@Khan you should really post a new question since you're effectively asking for help debugging your code! You do need to close every fd that the child uses, your code has an unrelated problem. In the commented-out line, you're closing an fd which the next iteration of the loop would be using! You should close all the fds in the parent - just not until you're done using them in the parent. If you had checked the return code of dup2 you'd have noticed (you also need to check the return codes of pipe/fork/close/execvp/waitpid).

[Asad-ullah Khan]

@NicholasWilson Ahh I see actually. And I apologize for posting code; I was not trying to seek help with it I just couldn't find a way to describe what the phenomenon that was occurring.