Shell pipe system in C

Question

I'm trying to make a pipe system for my shell, but it's not working as intended.

void pipes (char *listaCommand[], int end, char **argv)  
{  
    int cont = end;  
    for (cont;listaCommand[cont]; cont++)  
    {  
        if (listaCommand[cont] != NULL)  
        {
            if (!strcmp(listaCommand[cont],"|")){
                int pid2, status;
                int pipefd[2], ret;

                listaCommand[cont] = NULL;

                ret = pipe (pipefd);
                if (ret < 0) fatal();

                /* Now fork. */

                pid2 = fork ();
                if (pid2 <0) fatal ();

                if (pid2 > 0)
                {
                    printf ("P: waiting for child\n");
                    wait (&status);     
                    close(STDIN_FILENO);
                    dup(pipefd[0]);
                    close(pipefd[0]);
                    close(pipefd[1]);
                    /*execvp (auxCommand[0], auxCommand);*/
                    pipes(listaCommand, cont+1, argv);
                    /*break;*/
                }
                else
                {
                    close (STDOUT_FILENO);
                    dup (pipefd[1]);
                    close (pipefd[1]);  
                    close (pipefd[0]);

                }

            }
        }
    }
    if (end >= 3)
    {
        printf("%s \n", listaCommand[end-1]);
    }
    execvp (listaCommand[end], listaCommand);
    printf ("%s: command not found.\n", listaCommand[end]); /* Exec failed. */
    exit(EXIT_FAILURE);
}

If I use commands like ls | sort, it works, but if ls has any argument, it doesnt work, because for some reason, listaCommand[cont] where its == "|" is not NULL, so I just get ls: option -- 'a' invalid.

listaCommand have  
[0] = "ls"  
[1] = "-al"  
[2] = "|"  
[3] = "sort"  

Answer 1

You don't need to pass the end argument, instead increment the pointer to your command array. You are passing the initial array to the execvp call so it tries to execute ls multiple times. Further, you need a break statement after setting the listaCommand[cont] to NULL because after the iteration cont is incremented. Also I think you need to protect the execvp call so that the parent does not call it after the processing is done.

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/wait.h>
#define fatal() exit(1)
void pipes (char *listaCommand[], char **argv)  
{  
    printf("pipes %s\n", listaCommand[0]);
    int cont = 0;
    for (;listaCommand[cont]; cont++)  {  
        if (listaCommand[cont][0] == '|'){
            int pid2, status;
            int pipefd[2], ret;

            listaCommand[cont] = NULL;

            ret = pipe (pipefd);
            if (ret < 0) fatal();

            /* Now fork. */

            pid2 = fork ();
            if (pid2 <0) fatal ();

            if (pid2 > 0)
            {
                printf ("P: waiting for child\n");
                wait (&status);     
                close(STDIN_FILENO);
                dup(pipefd[0]);
                close(pipefd[0]);
                close(pipefd[1]);
                /*execvp (auxCommand[0], auxCommand);*/
                pipes(listaCommand + cont + 1, argv);
                /*break;*/
            }
            else
            {
                close (STDOUT_FILENO);
                dup (pipefd[1]);
                close (pipefd[1]);  
                close (pipefd[0]);
                break;
            }

        }
    }
    if (listaCommand[0]) {
      execvp (listaCommand[0], listaCommand);
      printf ("%s: command not found.\n", listaCommand[0]); /* Exec failed. */
      exit(EXIT_FAILURE);
    }
}

int main() {
    char *args[] = { "ls", "-al", "|", "sort", "|" , "tr", "[a-z]", "[A-Z]", 0 };
    pipes(args, 0);
    return 0;
}