Imitate unix shell pipe

Question

Educational task: want to imitate pipe symbol (command, method) "|" work. Program gets command like in unix shell from STDIN:

command1 | command2 | command3 | ....

And should execute it redirecting STDIN|STDOUT to a pipe for each command. Final output redirects to result.out file. Should use only execlp and fork.

1st variant: works fine for 1-2 commands, however freezes for 3 or more. What I'm doing wrong: seems I close all the pipe descriptors?

Now in 2nd variant execute_line was simplified, now another problem: mess in output. How to correctly pass pipes between commands?

3rd variant: most close to correct, added more debug info. Question: how to correctly connect middle children?

4th variant, fixed logic, almost correct: works fine with 1, 3 or more commands, start to fail with 2 (which worked correct previously) - weird :)

#include <stdio.h>
#include <unistd.h>
#include <iostream>
#include <fstream>
#include <algorithm>
#include <cstring>
#include <vector>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

using namespace std;

void split(const string& str, vector<string> &tokens,
           const string &delimiters = " ")
{
    // Skip delimiters at beginning.
    string::size_type lastPos = str.find_first_not_of(delimiters, 0);
    // Find first "non-delimiter".
    string::size_type pos     = str.find_first_of(delimiters, lastPos);

    while (string::npos != pos || string::npos != lastPos)
    {
        // Found a token, add it to the vector.
        tokens.push_back(str.substr(lastPos, pos - lastPos));
        // Skip delimiters.  Note the "not_of"
        lastPos = str.find_first_not_of(delimiters, pos);
        // Find next "non-delimiter"
        pos = str.find_first_of(delimiters, lastPos);
    }
}

inline string trim(string &str)
{
    const string whitespaces(" \t\f\v\n\r");

    string::size_type pos = str.find_first_not_of(whitespaces);
    if(pos != string::npos)
        str.erase(0, pos);   // prefixing spaces

    pos = str.find_last_not_of(whitespaces);
    if(pos != string::npos)
        str.erase(pos + 1);    // surfixing spaces

    return str;
}

void parse_command(string &command, string &name, string &argc)
{
    command = trim(command);

    string::size_type pos = command.find_first_of(' ');
    if(pos != string::npos) {
        name = command.substr(0, pos);
        argc = command.substr(pos + 1, command.length() - pos - 1);
    } else {
        name = command;
        argc = "";
    }
}

void exec_command(uint n, vector<string> &commands)
{
    string name, args;
    parse_command(commands[n], name, args);
    if(args.length() > 0)
        execlp(name.c_str(), name.c_str(), args.c_str(), NULL);
    else
        execlp(name.c_str(), name.c_str(), NULL);
}

// who ----(stdout)---> pfd[1] --- pfd[0] ----(stdin)---> wc -l
void execute_line(vector<string> &commands, uint i, int *parent_pfd = 0)
{
    int pfd[2];
    pipe(pfd);
    if(i > 0 && !fork()) {
        // Child
        printf("Child, i: %d\n", i);
        if(i > 1) {
            execute_line(commands, i-1, pfd);
            close(pfd[1]);
            close(pfd[0]);
        } else {
            printf("Deeper child %d: %s, parent_pfd[0]=%d, parent_pfd[1]=%d, "
                   "pfd[0]=%d, pfd[1]=%d\n",
                   getpid(), trim(commands[i-1]).c_str(),
                   parent_pfd[0], parent_pfd[1], pfd[0], pfd[1]);

            close(STDOUT_FILENO);

//            if(parent_pfd)
//                dup2(parent_pfd[1], STDOUT_FILENO); // Copy STDOUT to parent pipe out
//            else
                dup2(pfd[1], STDOUT_FILENO);        // Copy STDOUT to pipe out

            close(pfd[1]);
            close(pfd[0]);

            exec_command(i - 1, commands);
        }
    } else {
        if(parent_pfd) {
            printf("Middle Child, i: %d\n", i);
            printf("Middle child %d: %s, parent_pfd[0]=%d, parent_pfd[1]=%d, "
                   "pfd[0]=%d, pfd[1]=%d\n",
                   getpid(), trim(commands[i]).c_str(), parent_pfd[0], parent_pfd[1],
                   pfd[0], pfd[1]);

            close(STDIN_FILENO);
            dup2(pfd[0], STDIN_FILENO);         // Copy STDIN to pipe in

            close(STDOUT_FILENO);
            dup2(parent_pfd[1], STDOUT_FILENO); // Copy STDOUT to parent pipe out

            close(pfd[1]);
            close(pfd[0]);

            exec_command(i, commands);
        } else {
            printf("Final, i: %d\n", i);
            printf("Final %d: %s, pfd=%p, parent_pfd=%p, pfd[0]=%d, pfd[1]=file\n",
                   getpid(), trim(commands[i]).c_str(), pfd, parent_pfd, pfd[0]);
            int fd = open("result.out", O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
            dup2(fd, STDOUT_FILENO);            // Copy stdout to file
            dup2(pfd[0], STDIN_FILENO);         // Copy STDIN to pipe in
            close(pfd[0]);  // Close as was redirected
            close(pfd[1]);  // Close WRITE as not necessary here
            close(fd);

            exec_command(i, commands);
        }
    }
}

int main()
{
    char buffer[1024];
    ssize_t size = read(STDIN_FILENO, buffer, 1024);

    if(size > 0) {
        buffer[size] = '\0';
        string command = buffer;
        vector<string> commands;
        split(command, commands, "|");
        execute_line(commands, commands.size() - 1);
    }
    return 0;
}

Answer 1

The logic you use to connect the pipes to standard input and output looks faulty.

int pfd[2];
pipe(pfd);

You start by creating a pipe, presumably to connect one process's standard output to another process's standard input. That's fine.

Now, let's look at one of section of the code which is going to execute one of the processes:

close(STDIN_FILENO);
close(STDOUT_FILENO);
dup2(pfd[0], STDIN_FILENO);     // Copy STDIN to pipe in
dup2(pfd[1], STDOUT_FILENO);    // Copy STDOUT to pipe out
close(pfd[0]);  // Close as was redirected
close(pfd[1]);  // Close as was redirected
exec_command(i, commands);

Now, I don't even need to explain this. You can just read your own comments here, then try to explain why you are attaching both ends of the same pipe to the same process's standard input and output? That makes no sense. The pipe should be attaching one process's standard input to another process's standard output. In this kind of a situation, it makes no sense to execute a process and attach its standard input to standard output. That's a head scratcher to me.

That's one of the issues here, but there are probably a few others here too, that would be apparent after a closer look.

The overall approach seems too complicated to me, here. This recursive function, that sets up the pipeline, should really only have a single decision point: is this the last command in the pipeline. If so, do one thing. If not, then do something else that involves recursion, to set up the rest of the pipeline.

It looks to me like there are either three or four decision points here, so even if the overall logic here is slightly over-complicated, but not wrong, it should be simplified. You shouldn't have to do any special coding for the "middle" part of the pipeline, as your comments describe. Either you're dealing with the last command in the pipeline, or not. That's it. Try to rewrite your function in this manner. It should be much simpler, and work better.

Answer 2

I am used the next logic (pseudo code for my solution):

I first put together in a linked list of all the commands by assigning each respective descriptors, and then executed in order.

int pfd_cur[2];
int in = -1;
int out = -1;
while (number_of_commands) { // process all commands from first to last
    number_of_commands--; // and set cur_command_number
    if(cur_command_number == 0) {
        if(number_of_commands > 0) {
            pipe(pfd_cur);
            out = pfd_cur[1];
            in = pfd_cur[0];
            // We process first command
            // collect command (STDIN_FILENO, out);
        } else {
            // We process first command and we have only one command
            // collect command (STDIN_FILENO, STDOUT_FILENO);
        }
    } else {
        if(number_of_commands == 0)
        {
            // We process last command
            // collect command(in, STDOUT_FILENO);
        } else {
            pipe(pfd_cur);
            out = pfd_cur[1];
            // We process intermediate command
            // collect command (in, out);
            in = pfd_cur[0];
        }
    }
}

My struct for store connected process data in the linked list

struct node {
    // pipe num
    int in_fd;
    int out_fd;
    const char *command;
    char **argv;
    int agrc;
    struct node *next;
    struct node *prev;
};

My function to execute each stored node

void execute(struct node* command)
{
    if(!fork())
    {
        if(command->out_fd != 1)
        {
            close(STDOUT_FILENO);
            dup2(command->out_fd, STDOUT_FILENO);

        }
        if(command->in_fd != 0)
        {
            close(STDIN_FILENO);
            dup2(command->in_fd, STDIN_FILENO);
        }

        execvp(command->command, command->argv);

        close(command->out_fd);
        close(command->in_fd);
    }
}

Answer 3

In final variant just simple if expression (should be i >= 1) error. So correct execute_line() method variant is:

void execute_line(vector<string> &commands, size_t i, int *parent_pfd = 0)
{
    int pfd[2];
    pipe(pfd);
    if(i > 0 && !fork()) {
        // Child
        if(i >= 1) {
            execute_line(commands, i-1, pfd);
            close(pfd[1]);
            close(pfd[0]);
        } else {
            printf("Deeper child %d: %s, parent_pfd[0]=%d, parent_pfd[1]=%d, "
                   "pfd[0]=%d, pfd[1]=%d\n",
                   getpid(), trim(commands[i-1]).c_str(),
                   parent_pfd[0], parent_pfd[1], pfd[0], pfd[1]);

            close(STDOUT_FILENO);
            dup2(pfd[1], STDOUT_FILENO);        // Copy STDOUT to pipe out

            close(pfd[1]);
            close(pfd[0]);

            exec_command(i - 1, commands);
        }
    } else {
        if(parent_pfd) {
            printf("Middle child %d: %s, parent_pfd[0]=%d, parent_pfd[1]=%d, "
                   "pfd[0]=%d, pfd[1]=%d\n",
                   getpid(), trim(commands[i]).c_str(), parent_pfd[0], parent_pfd[1],
                   pfd[0], pfd[1]);

            close(STDIN_FILENO);
            dup2(pfd[0], STDIN_FILENO);         // Copy STDIN to pipe in

            close(STDOUT_FILENO);
            dup2(parent_pfd[1], STDOUT_FILENO); // Copy STDOUT to parent pipe out

            close(pfd[1]);
            close(pfd[0]);

            exec_command(i, commands);
        } else {
            printf("Final %d: %s, pfd=%p, parent_pfd=%p, pfd[0]=%d, pfd[1]=file\n",
                   getpid(), trim(commands[i]).c_str(), pfd, parent_pfd, pfd[0]);
            int fd = open("result.out", O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
            dup2(fd, STDOUT_FILENO);            // Copy stdout to file
            dup2(pfd[0], STDIN_FILENO);         // Copy STDIN to pipe in
            close(pfd[0]);  // Close as was redirected
            close(pfd[1]);  // Close WRITE as not necessary here
            close(fd);

            exec_command(i, commands);
        }
    }
}