Almost done linux shell pipe

Question

Hi i'm trying to build a shell on linux and i'm stuck with the pipelining part.First i take the inputs from the user like "ls | sort" then when i try to run the program it lookls like the commands ls and sort doesnt work It looks like i've done everything right but it still cant seem to work. can you help please. thanks in advance

include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
#include <stdlib.h>
#include <string.h>
#include <sys/param.h>
#include <fcntl.h>
#include <sys/stat.h>
#define CREATE_FLAGS (O_WRONLY | O_CREAT | O_APPEND)
#define CREATE_MODE (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
int setup();

int main(int argc, const char * argv[])
{

    while(1)

    {
        printf("333sh: ");
        if(setup())
            break;
    }

    return 0;
}
int setup(){




    char  input [128];
    char *arg[32];
    int i = 1;
    while(fgets(input,128,stdin)!=NULL)
    {
        arg[0] = strtok(input," \n");
        while((arg[i]=strtok(NULL," \n")) != NULL){
            i++;
            }



           if (arg[1]!=NULL && strcmp(arg[1],"|")==0 && arg[2]!=NULL ){
            pid_t pid;

            int fd[3];

        pipe(fd);

    pid=fork();
        if(pid<0){
            printf("fork");
        }
        else if(pid==0){
            pid_t cpid;

            cpid=fork();
                if(cpid==0){

                    dup2(fd[2], 1); // Replace stdin with the read end of the pipe
                    close(fd[0]); // Don't need another copy of the pipe read end hanging about
                    close(fd[2]);
                    execvp(arg[0],arg);
        }
                else if(pid>0){

                    dup2(fd[0], 0); // Replace stdout with the write end of the pipe
                    close(fd[0]); //close read from pipe, in parent
                    close(fd[2]); // Don't need another copy of the pipe write end hanging about
                    execvp(arg[2], arg);
        }
    }
        else if(pid>0){
            waitpid(pid, NULL,0);
  }

        }


            }





    }

score 1 · Answer 1 · answered Dec 06 '13 at 23:00

1

You're using fd[0] and fd[2] but pipe(fd) only sets fd[0] and fd[1].

answered Dec 06 '13 at 23:00

Ben Jackson

90,079
9
98
150

yes i realized that and changed fd[2] to fd[1] but the same error is still on – user3076248 Dec 06 '13 at 23:06

score 1 · Accepted Answer · edited May 23 '17 at 12:13

Your biggest problem is that your argument lists for your commands are malformed (after you've resolved the index 2 vs index 1 issue with the pipe file descriptors diagnosed by Ben Jackson in his answer).

I added a function:

static void dump_args(int pid, char **argv)
{
    int i = 0;
    fprintf(stderr, "args for %d:\n", pid);
    while (*argv != 0)
        fprintf(stderr, "%d: [%s]\n", i++, *argv++);
}

and called it just before the calls to execvp(), and the output I got was:

$ ./ns
333sh: ls | sort
args for 29780:
0: [ls]
1: [|]
2: [sort]
ls: sort: No such file or directory
ls: |: No such file or directory
^C
$

The control-C was me interrupting the program. The arguments for each command must be 'the command name' (conventionally, the name of the executable), followed by the remaining arguments and a null pointer.

Your tokenization code is not providing two correct commands.

You also have a problem with which PID you're looking at:

                cpid = fork();
                if (cpid == 0)
                {
                    dup2(fd[1], 1);
                    close(fd[0]);
                    close(fd[1]);
                    dump_args(getpid(), arg);
                    execvp(arg[0], arg);
                    fprintf(stderr, "Failed to exec %s\n", arg[0]);
                    exit(1);
                }
                else if (pid > 0)  // should be cpid!
                {
                    dup2(fd[0], 0);
                    close(fd[0]);
                    close(fd[1]);
                    dump_args(pid, arg);
                    execvp(arg[1], arg);
                    fprintf(stderr, "Failed to exec %s\n", arg[1]);
                    exit(1);
                }

You also need to close the pipe file descriptors in the parent process before waiting.

This code compiles and 'works' for simple x | y command sequences such as ls | sort or ls | sort -r. However, it is far from being a general solution; you'll need to fix your argument parsing code quite a lot before you reach a general solution.

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

int setup(void);

int main(void)
{
    while (1)
    {
        printf("333sh: ");
        if (setup())
            break;
    }
    return 0;
}

static void dump_args(int pid, char **argv)
{
    int i = 0;
    fprintf(stderr, "args for %d:\n", pid);
    while (*argv != 0)
        fprintf(stderr, "%d: [%s]\n", i++, *argv++);
}

int setup(void)
{
    char input[128];
    char *arg[32];
    int i = 1;
    while (fgets(input, sizeof(input), stdin) != NULL)
    {
        arg[0] = strtok(input, " \n");
        while ((arg[i] = strtok(NULL, " \n")) != NULL)
        {
            i++;
        }
        if (arg[1] != NULL && strcmp(arg[1], "|") == 0 && arg[2] != NULL)
        {
            pid_t pid;
            int fd[2];
            arg[1] = NULL;

            pipe(fd);

            pid = fork();
            if (pid < 0)
            {
                fprintf(stderr, "fork failed\n");
                return 1;
            }
            else if (pid == 0)
            {
                pid_t cpid = fork();
                if (cpid < 0)
                {
                    fprintf(stderr, "fork failed\n");
                    return 1;
                }
                else if (cpid == 0)
                {
                    printf("Writer: [%s]\n", arg[0]);
                    dup2(fd[1], 1);
                    close(fd[0]);
                    close(fd[1]);
                    dump_args(getpid(), arg);
                    execvp(arg[0], arg);
                    fprintf(stderr, "Failed to exec %s\n", arg[0]);
                    exit(1);
                }
                else
                {
                    printf("Reader: [%s]\n", arg[2]);
                    assert(cpid > 0);
                    dup2(fd[0], 0);
                    close(fd[0]);
                    close(fd[1]);
                    dump_args(getpid(), &arg[2]);
                    execvp(arg[2], &arg[2]);
                    fprintf(stderr, "Failed to exec %s\n", arg[2]);
                    exit(1);
                }
            }
            else
            {
                close(fd[0]);
                close(fd[1]);
                assert(pid > 0);
                while (waitpid(pid, NULL, 0) != -1)
                    ;
            }
        }
    }
    return 1;
}

score 0 · Answer 3 · edited Dec 07 '13 at 18:18

0

Couple of immediate problems:

setup() has no return value, but you expect an int
The definition of fgets is:
```
char * fgets ( char * str, int num, FILE * stream );
```
Get string from stream
Reads characters from stream and stores them as a C string into str until (num-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first.

A newline character makes fgets stop reading, but it is considered a valid character by the function and included in the string copied to str.

fgets() returns NULL on an error; otherwise it returns a pointer to str. So this seems like a very unsound test condition in your while loop.

edited Dec 07 '13 at 18:18

Jonathan Leffler

730,956
141
904
1,278

answered Dec 07 '13 at 00:20

modulus0

3,798
1
12
10

Your definition of `fgets()` differs in some details from the standard C definition of [`fgets()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fgets.html). Also, I'm not clear what your last paragraph is trying to claim: `fgets()` returns NULL on EOF or error — and `while (fgets(line, sizeof(line), fp) != NULL)` is a fine idiom for a loop that reads to EOF. – Jonathan Leffler Dec 07 '13 at 18:21

Almost done linux shell pipe

3 Answers3