CODEWITHSUNDEEP
c++pipeforkgetlinedup2
Sarah Allec
Sarah Allec

Reputation: 11

Coding pipe "|" in C++: Why is a blank line entering getline after piping?

I'm coding a mock shell and I'm currently working on coding pipes with dup2. Here is my code:

bool Pipe::execute() {
    int fds[2]; //will hold file descriptors
    pipe(fds);
    int status;
    int errorno;
    pid_t child;
    child = fork();

    if (-1 == child) {
        perror("fork failed");
    }
    if (child == 0) {
        dup2(fds[1], STDOUT_FILENO);
        close(fds[0]);
        close(fds[1]);
        this->component->lchild->execute();
        _exit(1);
    }
    else if (child > 0) {
        dup2(fds[0], STDIN_FILENO);
        close(fds[0]);
        close(fds[1]);
        this->component->rchild->execute();
        waitpid(child, &status, 0);
        if ( WIFEXITED(status) ) {
            //printf("child exited with = %d\n",WEXITSTATUS(status));
            if ( WEXITSTATUS(status) == 0) {
                cout << "pipe parent finishing" << endl;
                return true;
            }
        }
        return false;
    }
}

The this->component->lchild->execute(); and this->component->rchild->execute(); run execvp on the corresponding commands. I've confirmed that each of these return by printing out a statement in the parent process. However, in my Pipe::execute(), it seems like the child process does not finish because the cout statement in the parent process is never printed and I get a segmentation fault after the prompt ($) is initialized (see picture). Here is the main function that initializes the prompt after each execution:

int main()
{
    Prompt new_prompt;
    while(1) {
        new_prompt.initialize();
    }
    return 0;
}

and here is the initialize() function:

void Prompt::initialize()
{
    cout << "$ ";
    std::getline(std::cin, input);

    parse(input);
    run();
    input.clear();
    tokens.clear();
    fflush(stdout);
    fflush(stdin);
    return;
}

It seems like ls | sort runs fine, but then when the prompt is initialized, a blank line is read into input by getline. I've tried using cin.clear(), cin.ignore, and the fflush and clear() lines above. This blank string is "parsed" and then the run() function is called, which tries to dereference a null pointer. Any ideas on why/where this blank line is being entered into getline? and how do I resolve this? Thank you!

UPDATE: The parent process in the pipe is now finishing. I've also noticed that I'm getting seg faults also for my I/O redirection classes (> and <). I think I'm not flushing the stream or closing the file descriptors correctly...

Here is my execute() function for lchild and rchild:

bool Command::execute() {
    int status;
    int errorno;
    pid_t child;
    vector<char *> argv;
    for (unsigned i=0; i < this->command.size(); ++i) {
        char * cstr = const_cast<char*>(this->command.at(i).c_str());
        argv.push_back(cstr);
    }
    argv.push_back(NULL);
    child = fork();
    if (-1 == child) {
        perror("fork failed");
    }
    if (child == 0) {
        errorno = execvp(*argv.data(), argv.data());
        _exit(1);

    } else if (child > 0) {
        waitpid(child, &status, 0);
        if ( WIFEXITED(status) ) {
            //printf("child exited with = %d\n",WEXITSTATUS(status));
            if ( WEXITSTATUS(status) == 0) {
                //cout << "command parent finishing" << endl;
                return true;
            }
        }
        return false;
    }
}

Upvotes: 1

Views: 306

Answers (1)

Michael Veksler
Michael Veksler

Reputation: 8475

Here is the bug:

else if (child > 0) {
    dup2(fds[0], STDIN_FILENO);
    close(fds[0]);
    close(fds[1]);
    this->component->rchild->execute();

You are closing stdin for the parent, not just the right child. After this the stdin of the parent process is the same as that of the right child.

After that

std::getline(std::cin, input);

Tries to read the output of the left child, rather than the original stdin. By that point the left child had finished and that end of the pipe had been closed. This makes reading stdin fail, and leave input unchanged in its original state.

Edit: Your design has a minor and a major flaws. The minor flaw is that you don't need the fork in Pipe::execute. The major flaw is that the child should be the one who redirects streams and closes the descriptors.

Simply pass input and output parameters through fork() and let the child dup2 these. Don't forget to make it also close unrelated pipe ends. If you don't, the left child will finish but its output pipe will continue living in other processes. As long as other copies of that descriptor live, the right child will never get EOF while reading its pipe end - and week block forever.

Upvotes: 2

Related Questions