3
\$\begingroup\$

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 <sys/types.h> 
#include <errno.h> 
#include <stdio.h> 
#include <stdlib.h> 
#include <unistd.h> 
#include <string.h>
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 < n - 1; ++i) {
        pipe (fd);
        spawn_proc (in, fd [1], cmd + i);
        close (fd [1]);
        in = fd [0];
    }
    if (in != 0)
        dup2 (in, 0);
    return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);
}

int main (int argc, char ** argv) {
    int i;
    if (argc == 1) { /* There were no arguments */
        const char *printenv[] = { "printenv", 0};
        const char *sort[] = { "sort", 0 };
        const char *less[] = { "less", 0 };
        struct command cmd [] = { {printenv}, {sort}, {less} };
        return fork_pipes (3, cmd);
    }
    if (argc > 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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Are you making your own OS, or are you making your own command line environment? BASH is a command line environment that runs on the Linux OS. \$\endgroup\$ – KeithSmith Apr 5 '15 at 18:25
  • \$\begingroup\$ @KeithSmith Both, but this is only the command line environment. The os parts are a separate project. \$\endgroup\$ – Niklas Rosencrantz Apr 5 '15 at 20:28
4
\$\begingroup\$
  • 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 < n - 1 loop terminating condition looks alarming. It takes some mental effort to understand what you are after. You may try to heal it with the comment, but most likely you should treat all pipeline components equally.

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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.