c - Multiple pipe implementation using system call fork() execvp() wait() pipe() - it is simply not working

Question

I need to implement my shell that handles multiple pipe commands. For example I need to be able to handle this: ls | grep -i cs340 | sort | uniq | cut -c 5. I am assuming the problem is that I am not passing output of the previous command to the input of the next command. When I execute my code, it gives me no output. I am using this pseudo code:

for cmd in cmds
    if there is a next cmd
        pipe(new_fds)
    fork
    if child
        if there is a previous cmd
            dup2(old_fds[0], 0)
            close(old_fds[0])
            close(old_fds[1])
        if there is a next cmd
            close(new_fds[0])
            dup2(new_fds[1], 1)
            close(new_fds[1])
        exec cmd || die
    else
        if there is a previous cmd
            close(old_fds[0])
            close(old_fds[1])
        if there is a next cmd
            old_fds = new_fds
if there are multiple cmds
    close(old_fds[0])
    close(old_fds[1])

Here is the source code of the function that handles multiple pipes.

void execute_multiple_commands(struct command ** commands_to_exec,
        int num_commands_p)
{
    pid_t status;
    int i, err;
    int new_fd[2], old_fd[2];
    pid_t pid, cpid;

    // creating child process
    if ( (cpid = fork()) == -1)
    {
        fprintf(stderr, "Could not create child process, exiting...");
        exit(1);
    }

    if (cpid == 0) // in the child process we run multiple pipe handling
    {
        for (i = 0; i < num_commands_p; i++) // for each cmd in cmds
        {
            if (i+1 < num_commands_p) // if there is next cmd
                pipe(new_fd);

            if ( (pid = fork()) == -1)
            {
                fprintf(stderr, "Could not create child process, exiting...");
                exit(1);
            }

            if (pid == 0) // if child
            {
                if (i != 0) // if there is a previous command
                {
                    dup2(old_fd[0], 0); // setting up old_pipe to input into the child
                    close(old_fd[0]);
                    close(old_fd[1]);

                }
                if (i+1 < num_commands_p) // if there is a next cmd
                {
                    close(new_fd[0]); // setting up new_pipe to get output from child
                    dup2(new_fd[1], 1);
                    close(new_fd[1]);

                    err = execvp(commands_to_exec[i]->args[0], commands_to_exec[i]->args);
                    status = err;
                    exit(err);
                }
            }
            else
            {
                waitpid(pid, &status, 0);
                if (status == -1)
                    exit(1);

                if (i != 0) // if there a previous command
                {
                    close(old_fd[0]);
                    close(old_fd[1]);
                }
                if (i+1 < num_commands_p) // if there a next cmd
                {
                    old_fd[0] = new_fd[0];
                    old_fd[1] = new_fd[1];
                }
                exit(0);
            } // end if
        } // end for

        if (i) // if there a multiple commands
        {
            close(old_fd[0]);
            close(old_fd[1]);
        }
    }
    else // in the parent process we are waiting for child to handle multiple pipes
        waitpid(cpid, &status, 0);
}

Function execvp() takes array of structures. Ive checked all my parsing part, and it works fine. It is the execute_multiple_commands() function that I am having trouble with.

Here is the code for struct:

// name: command
// desc: holds one command (meaning that it can be
//        more than one token in that command)
//        "ls -la" will be an example of one command
//       holds num of tokens in command array
struct command
{
    char ** args;
    int num_args;
};

Answer 1

I suggest a new strategy, R2:

function do(commands)
    if commands is of size 1
        exec commands[0] || die
    split commands into c1 (first command) c2 (the rest of them)
    open
    if fork 
        close input end of pipe
        dup output of pipe to stdin
        do (c2) || die
    close output end of pipe
    dup input of pipe to stdout
    exec c1 || die

Using a recursive function, especially if you're maintaining a list, will help you simplify your logic. You don't really have to worry about stack depth here, as your whole address space will be overwritten anyway.

In other news, from the man page:

After a successful return from one of these system calls, the old and new file descriptors may be used interchangeably. They refer to the same open file description (see open(2)) and thus share file offset and file status flags; for example, if the file offset is modified by using lseek(2) on one of the descriptors, the offset is also changed for the other.

Which meanse when you say you're closing both ends of the pipe? You really are closing it - it AND the standard in/out that your program is intending on using.

--> MUCH LATER EDIT <--

As Jonathan Leffler pointed out, the above information is in correct. I have confirmed it with the following program:

#include <unistd.h>

int main(){
    dup2(0, 7);
    write(7, "Hey, 1
", 7);
    close(0);
    write(7, "Hey, 2
", 7);
    close(7);
    write(7, "Hey, 3
", 7);
}

Which results in the following output:

$ gcc dup2Test.c && ./a.out
Hey, 1
Hey, 2

Thanks, Jonathan!