Writing my own shell... stuck on pipes?

Question

For the past few days I have been attempting to write my own shell implementation but I seem to have gotten stuck on getting pipes to work properly. I am able to parse a line and fork off the commands between the pipes (ex: ls | sort) individually but can't seem to get them to pipe input from one into the other.

I think I just don't understand how to use dup2() and pipes properly.

I've now included my code which is still failing... :( So stuck...

void forkAndExecute( char* arrayOfWords[] , vector<pid_t> *vectorOfPIDs , bool hasNextCmd , bool hasPrevCmd) {

int fd[ 2 ];
pid_t pid;

if( hasNextCmd ){
    pipe(fd);
}

pid = fork();

//error if PID < 0
if( pid < 0 ) {
    cerr << ">>> fork failed >>>" << endl;
    exit(-1);
}
//child process if PID == 0
else if( pid == 0 ) {
    if ( hasPrevCmd ){
        dup2(fd[0] , 0);
        close(fd[0]);
        close(fd[1]);

    }
    if ( hasNextCmd ){
        dup2(fd[1],1);
        close(fd[0]);
        close(fd[1]);
    }
    execvp( arrayOfWords[0] , arrayOfWords );
    cout << ">>> command not found >>>" << endl;
    //if logic reaches here, exec failed
    exit(0);
} 
//parent process
else{
    close(fd[0]);
    close(fd[1]);
    //if( ! isLastCmd ){

    //}
    vectorOfPIDs->push_back(pid);
}

}

Answer 1

First suggestion: Symbolic constants are better than magic numbers.

const int PIPE_READ = 0;
const int PIPE_WRITE = 1;
int fd[2];
pipe(fd);
// Now you can refer to fd[PIPE_READ] and fd[PIPE_WRITE].

Second suggestion: Take a step back and think about what you're trying to accomplish.

You want to spawn two processes, with the first process's stdout connected to the second process's stdin. Right?

So, in C, this means that you need to take call pipe, pass fd[PIPE_WRITE] to the first child process, which will dup2 it to 1, and pass fd[PIPE_READ] to the second child process, which will dup2 it to 0.

Simply looking at forkAndExecute's prototype shows that it can't do that:

void forkAndExecute( char* arrayOfWords[] , vector *vectorOfPIDs , 
    bool hasNextCmd , bool hasPrevCmd);

It only handles a single command, and from looking at that argument list, unless it resorts to evil global variables, there's no way for it to receive a file descriptor from its PrevCmd or receive a file descriptor from its NextCmd.

Think about how to manage the file descriptors that you need, and redesign forkAndExecute to be able to use these.