HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Simple Linux pipeline

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplepipelinelinux

Problem

I've compiled a simple Linux pipeline and using my C style and modularization that I will use for my own operating system. Do you have something to say about this code (partly taken from SO)? For example, are all my imports necessary? How can I improve error handling?

#include  
#include  
#include  
#include  
#include  
#include 
struct command
{
    const char **argv;
};

int spawn_proc (int in, int out, struct command *cmd) {
    pid_t pid;
    if ((pid = fork ()) == 0) {
        if (in != 0) {
            dup2 (in, 0);
            close (in);
        }
        if (out != 1) {
            dup2 (out, 1);
            close (out);
        }
        return execvp (cmd->argv [0], (char * const *)cmd->argv);
    }
    return pid;
}

int fork_pipes (int n, struct command *cmd) {
    int i;
    pid_t pid;
    int in, fd [2];
    in = 0;
    for (i = 0; i  1) { /* I'd like an argument */
        char *tmp;
        int len = 1; 
        for( i=1; i<argc; i++)
        {
            len += strlen(argv[i]) + 2; 
        }
        tmp = (char*) malloc(len);
        tmp[0] = '\0';
        int pos = 0;
        for( i=1; i<argc; i++)
        {
            pos += sprintf(tmp+pos, "%s%s", (i==1?"":"|"), argv[i]);
        }
        const char *printenv[] = { "printenv", 0};
        const char *grep[] = { "grep", "-E", tmp, NULL};
        const char *sort[] = { "sort", 0 };
        const char *less[] = { "less", 0 };
        struct command cmd [] = { {printenv}, {grep}, {sort}, {less} };
        return fork_pipes (4, cmd);
        free(tmp);
    }
}


What it does is simply a printenv | sort | less if there are no args and if there are arguments it is taken as parameter list to grep to grep for environment varibles. I'm using it to develop my programming and make my own command-line environment for my own FPGA operating system.

Solution

-
dup2 handles equal file descriptors very nicely. There is no need to test for in != 0 etc.

-
On the other hand, you must check the return values, especially those of the system calls, and give a proper diagnostics when necessary.

-
An idea of executing the last pipeline component inside a master process is dubious. You lose the control over children's lifecycle. I recommend to run each component in its own process, and wait for all of them.

An i

-
Keep in mind that some commands (
cd` being most prominent) must be builtins. Spawning a shell for them would be a design mistake.

Context

StackExchange Code Review Q#85936, answer score: 4

Revisions (0)

No revisions yet.