Pipes not working while implementing a minishell

Question

I'm actually trying to implement a basic minishell in C. For doing that, I made a fonction which parse what the user enter in the console. I parsed it and then I would like to send the command to the console using pipes. I don't understand why my pipes are not working. I checked the parsing and it seems to be fine, i have the right commands in parameters of the fonction. The thing is that my pipe code is literally doing nothing and I don't understand why. Here is my code. Thank you in advance.

#define READ 0
#define WRITE 1

char *cmds[5] = {0};

int main() {    
    char *saisie = (char*)malloc(100*sizeof(char*));
    char *saisie2 = (char*)malloc(100*sizeof(char*));
    gets(saisie);
    int ncmds = 0;
    int k = 0;

    char* token = (char*)malloc(100*sizeof(char*));
    char* tofree;

    if(*(saisie + 0) == '$'){
        if(*(saisie + 2) == 'e' && *(saisie + 3) == 'x' && *(saisie + 4) == 'i' || *(saisie + 5) == 't'){
            exit(0);
        }
        else{
            int i;
            for(i = 0;i<99;i++){
                *(saisie2+i) = *(saisie+i+1);
            }       
            free(saisie);

            if (saisie2 != NULL) {
                tofree = saisie2;

                while ((token = strsep(&saisie2, "|")) != NULL){
                     cmds[ncmds] = token;
                     ncmds++;
                }
                free(tofree);           
            }
        }
    }

    exe(cmds, ncmds);   
    while(wait(NULL) > 0);
    return 0;
}

int exe(char *cmds[], int ncmds){
    int fdin, fdout;
int fds[2];
int i;
int status;
fdin = 0;
for(i=0; i < ncmds-1; i++){
    pipe(fds);
    fdout = fds[WRITE];

    if(fork() == 0){
        if( fdin != 0 ) {
            close(0);
            dup(fdin); 
            close(fdin);
        }
        if( fdout != 1 ) {
            close(1);
            dup(fdout); 
            close(fdout);
        }
        close(fds[READ]);
        const char* prog2[] = {cmds[i], "-l", 0};
        execvp(cmds[i], prog2);
        fprintf(stderr, "si esto se ve es un error\n");
        exit(1);
    }

    if(fdin != 0)
        close(fdin);
    if(fdout != 1)
        close(fdout);

    fdin = fds[READ];
}

/* Ultimo comando */
fdout = 1;
if(fork() == 0) {
    if( fdin != 0 ) {
        close(0); 
        dup(fdin); 
        close(fdin);
    }
    const char* prog2[] = {cmds[i], "-l", 0};
    execvp(cmds[i], prog2);
    close(fds[READ]);
    exit(1);
}

if(fdout!= 1)
    close(fdout);
if(fdin != 0)
    close(fdin);

    }
}

Never use `gets()`. For the love of `$DEITY`, *never use `gets()`*! — Adam Rosenfield, Jun 13 '13 at 02:55
Well, I used some print fonction to verify that the program enters well in the different condition and indeed it does. The problem seem to come from some wrong link with the dup, or the closes... — pdilau, Jun 13 '13 at 02:55
Offtopic too, but you don't need all those malloc, you could certainly refactor the code to use the stack. Also, your for-loop can be replaced by a call to strcpy (or strdup, both using `saisie+1`) but maybe you don't even need that and could do a `saisie2 = saisie + 1`. And finally, `*(saisie + 3)` can also be written `saisie[3]` which is more clear. — Maxime Chéramy, Jun 13 '13 at 06:19
[Don´t cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). — unwind, Jun 13 '13 at 08:54

leonhart · Accepted Answer · 2013-06-13T08:50:22.463

2

int exe(char *cmds[], int ncmds){
    int p2c[2];//pipe parent to child
    int c2p[2];//pipe child to parent
    int i;
    int status;
    int pid;
    char buf[4096];
    memset(buf, 0, 4096);

    for(i=0; i < ncmds; i++){
        pipe(p2c);
        pipe(c2p);

        pid = fork();
        if(pid < 0) {
            exit 1;
        }
        if(pid == 0){ //in child
            close(1);
            dup2(c2p[1],1); // make child write to c2p pipe instead of stdout
            close(0);
            dup2(p2c[0],0); // make child read from p2c pipe instead of stdin
            close(p2c[1]);
            close(c2p[0]);
            const char* prog2[] = {cmds[i], "-l", 0};
            execvp(cmds[i], prog2);
            fprintf(stderr, "si esto se ve es un error\n");
            exit(1);
        }
        //in parent
        write(p2c[1], buf, strlen(buf)); //write the last output to child
        close(p2c[1]);
        close(c2p[1]);
        memset(buf,0,4096);
        if(read(c2p[0], buf, 4096) > 0){ //read output from child
            if(i == ncmds -1 ){
                printf("result:\n");
                printf("%s\n", buf);
            }
        }
    }
}

edited Jun 13 '13 at 08:50

answered Jun 13 '13 at 03:03

leonhart

1,193
6
12

I don't really understand what the parent should do. I don't really understand how to send the information towards the console. I'm sorry for those questions which might seem really simple but I'm beginning in C... – pdilau Jun 13 '13 at 03:13
@user2480503 in child process, you made the fd of 1 (the stdout) to the pipe, so the output of `execvp(cmds[i], prog2);` which should show in console now goes to the pipe (and won't show in console). then in the parent process, you must read the output from the pipe and print it to console. – leonhart Jun 13 '13 at 03:18
So am I suppose to use something like that in the parent : system(fds[READ]). What I understood is that the fonctions of the exec family were suppose to execute a command the same way as if we execute them in the console. I did understand that I don't read the output of the pipe in the parent. Am I supposed to send a fonction which aim is to send an instruction to the console, to the console. I don't know if I explained clearly my misunderstood... – pdilau Jun 13 '13 at 03:27
@user2480503 if you use `system()`, you will not need the whole thing about pipe and `exec`, but it's not a good idea. yes the execl is suppose to do the samething as you execute from console. and the same thing is execute the command and output to fd 1 which should be stdout(the console) in common. but in your code, before `exec`, you redirected the fd 1 to your pipe, so output goes to fd 1 won't goes to console but your pipe. – leonhart Jun 13 '13 at 03:38
@user2480503 if you just want the output goes to console, maybe you don't need pipe. just fork a child process and call `exec` in it, I think it will do what you want. – leonhart Jun 13 '13 at 03:44
I understood your point. But I have this homework to do where it is explicitly asked to use pipe to implements the minishell. So that is what I am trying to do. I don't get how to take the command out of the pipe to redirect it toward the console... – pdilau Jun 13 '13 at 04:16
Is there a way to redirect directly the output of the pipe to the console? – pdilau Jun 13 '13 at 04:18
@user2480503 check my update, since the output goes to pipe, in parent process you can read the content from your pipe and then use printf or something to print it to console. – leonhart Jun 13 '13 at 04:38
So I'm trying to do something like that in the parent : close(fdout); printf(stdout, read(fdin, buf, sizeof(buf))); wait(&status); But isn't the fonction printf just to write something in the console that won't be executed? – pdilau Jun 13 '13 at 04:39
I read your update and I'm really not understanding what you're doing in the father... Sorry for the disturbing, I'm really lost here with that notion, and I have really no clue about it. – pdilau Jun 13 '13 at 05:15
@user2480503 I add some comment to my answer, hope this will help you . but it's really hard to explain farther with my poor english, sorry :( – leonhart Jun 13 '13 at 05:48
No that's fine, that's really understandable, no worries. Thank you very much for your help. BUt the fonction read always return me a negative number, i don't know why. I'm really in a dead end with this problem. – pdilau Jun 13 '13 at 05:53
@user2480503 sorry, i think i had misunderstand what's your problem. i will try to give another answer later. – leonhart Jun 13 '13 at 06:35
@user2480503 I updated the answer, this should solve your problem. – leonhart Jun 13 '13 at 08:50
Yes, thank you. It helped me for a part of the work I had to do, and help me to understand the mechanism as well ;). – pdilau Jun 13 '13 at 22:59

Pipes not working while implementing a minishell

1 Answers1