Search code examples
cpipeforkexecvpdup2

Having trouble connecting multiple pipes together


I'm trying to create a small shell program that can take multiple commands and chain them together and finally execute them.

As of now I have created a list to store the input and saved them to an array called command where I've split the input up using strtok and put the result in an array. So if I type in cat -n filename.txt I've put cat at command[0] -n at command[1] and filename.txt at command[2]. I've also added NULL at the end of every command, (ends up at command[3] in my cat example).

I'm able to execute the first command but if I try with a second command the first command is the only one to be executed so I'm suspecting that my pipes aren't linked correctly.

This is what I've done as of now:

  pid_t pid;

    int status;
    int pipes[number_arguments - 1][2];

    // Create pipes
    for (int i = 0; i < number_arguments - 1; i++)
    {
        if (pipe(pipes[i]))
        {
            perror("pipe");
            exit(EXIT_FAILURE);
        }
    }

    // Create new processes
    for (int i = 0; i < number_arguments; i++)
    {
        pid = fork();

        if (pid == -1)
        {
            perror("fork");
            exit(EXIT_FAILURE);
        }

        if (pid == 0)
        {
             // First child
            if (i == 0)
            {
                dup2(pipes[i][WRITE_END], STDOUT_FILENO);
            }

            //Middle child
            if ((i != 0) && ((i+1) != number_arguments))
            {
                dup2(pipes[i-1][READ_END], STDIN_FILENO);
                dup2(pipes[i][WRITE_END], STDOUT_FILENO);
            }

            // Last child
            if ((i+1) == number_arguments)
            {
                dup2(pipes[i][WRITE_END], STDOUT_FILENO);
            }

            // close all pipes
            for (int j = 0; j < number_arguments - 1; j++)
            {
                close(pipes[j][READ_END]);
                close(pipes[j][WRITE_END]);
            }

            if (execvp(command[0], command) < 0)
            {
                perror("exec failure");
                exit(EXIT_FAILURE);
            }
        }
        // Parent
        else
        {
            for (int j = 0; j < number_arguments - 1; j++)
            {
                close(pipes[j][READ_END]);
                close(pipes[j][WRITE_END]);
            }
            waitpid(pid, &status, 0);
        }
    }

I'm thankful for any kind of help I can get.

Sorry if I made som errors posting this, it's my first post.


Solution

  • You correctly create the pipes outside the command launching loop, and you correctly close all the pipe file descriptors in the children. The latter, in particular, is a common problem.

    However, you incorrectly have the parent process close all the pipes after launching the first child, so the second and subsequent kids have no pipes to work with. You also wait for each child in the pipeline to die before launching the next — you need the children to run concurrently.

    You need the parent process to launch all the child processes and only then wait for the pipeline to complete.

    The parent process will (probably) need to keep a record of all its children's PIDs. It must not close its copies of the pipes until all the children are launched (but it is critical that it then does so). It will need to use a loop around waitpid(). It must wait until the last child (the last process in the pipeline) terminates. It might or might not wait until all the earlier children in the pipeline terminate as well.

    One thing you should do, especially when things are going wrong, is error check the return values from system calls. You'd find that the dup2() and close() calls fail on the second iteration with EBADF — bad file descriptor. Even if things are working, it's not a bad idea to check the system calls and come up with an appropriate strategy if they fail.