Shell program with pipes in C

Question

I have a problem with pipes. My program is a Shell program in C. I want to execute for example ls | wc, but what I get after running is:

ls: cannot access |: no such file or directory ls: cannot access wc: no such file or directory.

What am I doing wrong?

#include <stdlib.h>   
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>

#define MAX_CMD_LENGTH 100

#define MAX_NUM_PARAMS 10

int parsecmd(char* cmd, char** params) { //split cmd into array of params
    int i,n=-1;
    for(i=0; i<MAX_NUM_PARAMS; i++) {
        params[i] = strsep(&cmd, " ");
        n++;
        if(params[i] == NULL) break;
    }
    return(n);
};

int executecmd(char** params) {
    pid_t pid = fork(); //fork process

    if (pid == -1) { //error
        char *error = strerror(errno);
        printf("error fork!!\n");
        return 1;
    } else if (pid == 0) { // child process
        execvp(params[0], params); //exec cmd
        char *error = strerror(errno);
        printf("unknown command\n");
        return 0;
    } else { // parent process
        int childstatus;
        waitpid(pid, &childstatus, 0);    
        return 1;
    }
};

int execpipe (char ** argv1, char ** argv2) {
    int fds[2];
    pipe(fds);
    int i;
    pid_t pid = fork();
    for (i=0; i<2; i++) {
        if (pid == -1) { //error
            char *error = strerror(errno);
            printf("error fork!!\n");
            return 1;
         } else
             if (pid == 0) {
                 if(i ==0){
                     close(fds[1]);
                     dup2(fds[0], 0);
                     close(fds[0]);
                     execvp(argv1[0], argv1);
                     char *error = strerror(errno);
                     printf("unknown command\n");
                     return 0;
                 } else if(i == 1) { 
                     close(fds[0]);
                     dup2(fds[1], 1);
                     close(fds[1]);
                     execvp(argv2[0], argv2);
                     char *error = strerror(errno);
                     printf("unknown command\n");
                     return 0;
                 }
            } else { // parent process
                int childstatus;
                waitpid(pid, &childstatus, 0);
                return 1;
            }
    } // end for
};


int main() {    
    char cmd[MAX_CMD_LENGTH+1];    
    char * params[MAX_NUM_PARAMS+1];    
    char * argv1[MAX_NUM_PARAMS+1];    
    char * argv2[MAX_NUM_PARAMS+1];    
    int k, y, x;    
    int f = 1;    
    while(1) {
        printf("$"); //prompt    
        if(fgets(cmd, sizeof(cmd), stdin) == NULL) break; //read command, ctrl+D exit       
        if(cmd[strlen(cmd)-1] == '\n') { //remove newline char    
            cmd[strlen(cmd)-1] = '\0';    
        }    
        int j=parsecmd(cmd, params); //split cmd into array of params           
        if (strcmp(params[0], "exit") == 0) break; //exit   
        for (k=0; k <j; k++) { //elegxos gia uparksi pipes    
            if (strcmp(params[k], "|") == 0) {    
                f = 0; y = k;      
               printf("pipe found\n");
            }               
        }
        if (f==0) {
            for (x=0; x<k; x++) {    
               argv1[x]=params[x];
            }     
            int z = 0;     
            for (x=k+1; x< j; x++) {     
                argv2[z]=params[x];
                z++;
            }     
            if (execpipe(argv1, argv2) == 0) break;    
         } else if (f==1) {     
             if (executecmd(params) == 0) break;
         }
    } // end while
    return 0;
}

Solution 1

Updated your code with following corrections.

1. Removed for() loop that iterated two times after fork() call.
2. Removed incorrect close of pipe FDs after dup2 calls for both parent and child processes.
3. Aligned the command that needed to be run as per the file descriptors that were duplicated in dup2() calls for parent and child. Basically I needed to swap execvp(argv2[0], argv2) and execvp(argv1[0], argv1) calls.
4. Added a break; statement in the for loop that searched for pipe character.

The updated code is as below.

#include <stdlib.h>   
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/wait.h>

#define MAX_CMD_LENGTH 100

#define MAX_NUM_PARAMS 10

int parsecmd(char* cmd, char** params) { //split cmd into array of params
    int i,n=-1;
    for(i=0; i<MAX_NUM_PARAMS; i++) {
        params[i] = strsep(&cmd, " ");
        n++;
        if(params[i] == NULL) break;
    }
    return(n);
};

int executecmd(char** params) {
    pid_t pid = fork(); //fork process

    if (pid == -1) { //error
        char *error = strerror(errno);
        printf("error fork!!\n");
        return 1;
    } else if (pid == 0) { // child process
        execvp(params[0], params); //exec cmd
        char *error = strerror(errno);
        printf("unknown command\n");
        return 0;
    } else { // parent process
        int childstatus;
        waitpid(pid, &childstatus, 0);    
        return 1;
    }
};

int execpipe (char ** argv1, char ** argv2) {
    int fds[2];
    pipe(fds);
    int i;
    pid_t pid = fork();
    if (pid == -1) { //error
        char *error = strerror(errno);
        printf("error fork!!\n");
        return 1;
    } 
    if (pid == 0) { // child process
        close(fds[1]);
        dup2(fds[0], 0);
        //close(fds[0]);
        execvp(argv2[0], argv2); // run command AFTER pipe character in userinput
        char *error = strerror(errno);
        printf("unknown command\n");
        return 0;
    } else { // parent process
        close(fds[0]);
        dup2(fds[1], 1);
        //close(fds[1]);
        execvp(argv1[0], argv1); // run command BEFORE pipe character in userinput
        char *error = strerror(errno);
        printf("unknown command\n");
        return 0;
    }
};


int main() {    
    char cmd[MAX_CMD_LENGTH+1];    
    char * params[MAX_NUM_PARAMS+1];    
    char * argv1[MAX_NUM_PARAMS+1] = {0};    
    char * argv2[MAX_NUM_PARAMS+1] = {0};    
    int k, y, x;    
    int f = 1;    
    while(1) {
        printf("$"); //prompt    
        if(fgets(cmd, sizeof(cmd), stdin) == NULL) break; //read command, ctrl+D exit       
        if(cmd[strlen(cmd)-1] == '\n') { //remove newline char    
            cmd[strlen(cmd)-1] = '\0';    
        }    
        int j=parsecmd(cmd, params); //split cmd into array of params           
        if (strcmp(params[0], "exit") == 0) break; //exit   
        for (k=0; k <j; k++) { //elegxos gia uparksi pipes    
            if (strcmp(params[k], "|") == 0) {    
                f = 0; y = k;      
               printf("pipe found\n");
               break;
            }               
        }
        if (f==0) {
            for (x=0; x<k; x++) {    
               argv1[x]=params[x];
            }     
            int z = 0;     
            for (x=k+1; x< j; x++) {     
                argv2[z]=params[x];
                z++;
            }     
            if (execpipe(argv1, argv2) == 0) break;    
         } else if (f==1) {     
             if (executecmd(params) == 0) break;
         }
    } // end while
    return 0;
}

If you are interested only in changes I made, here is the diff between your code and the above updated code:

--- original.c
+++ updated.c
@@ -4,6 +4,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <sys/types.h>
+#include <sys/wait.h>

 #define MAX_CMD_LENGTH 100

@@ -43,44 +44,36 @@
     pipe(fds);
     int i;
     pid_t pid = fork();
-    for (i=0; i<2; i++) {
         if (pid == -1) { //error
             char *error = strerror(errno);
             printf("error fork!!\n");
             return 1;
-         } else
-             if (pid == 0) {
-                 if(i ==0){
+    } 
+    if (pid == 0) { // child process
                      close(fds[1]);
                      dup2(fds[0], 0);
-                     close(fds[0]);
-                     execvp(argv1[0], argv1);
+        //close(fds[0]);
+        execvp(argv2[0], argv2); // run command AFTER pipe character in userinput
                      char *error = strerror(errno);
                      printf("unknown command\n");
                      return 0;
-                 } else if(i == 1) { 
+    } else { // parent process
                      close(fds[0]);
                      dup2(fds[1], 1);
-                     close(fds[1]);
-                     execvp(argv2[0], argv2);
+        //close(fds[1]);
+        execvp(argv1[0], argv1); // run command BEFORE pipe character in userinput
                      char *error = strerror(errno);
                      printf("unknown command\n");
                      return 0;
                  }
-            } else { // parent process
-                int childstatus;
-                waitpid(pid, &childstatus, 0);
-                return 1;
-            }
-    } // end for
 };


 int main() {    
     char cmd[MAX_CMD_LENGTH+1];    
     char * params[MAX_NUM_PARAMS+1];    
-    char * argv1[MAX_NUM_PARAMS+1];    
-    char * argv2[MAX_NUM_PARAMS+1];    
+    char * argv1[MAX_NUM_PARAMS+1] = {0};    
+    char * argv2[MAX_NUM_PARAMS+1] = {0};    
     int k, y, x;    
     int f = 1;    
     while(1) {
@@ -95,6 +88,7 @@
             if (strcmp(params[k], "|") == 0) {    
                 f = 0; y = k;      
                printf("pipe found\n");
+               break;
             }               
         }
         if (f==0) {

Solution 2

execv* procedure doesn't interpret shell script string. It merely starts an executable file and passes an array of arguments to it. Thus, it cannot organize a pipeline.

If you need "normal" shell command execution, you may want to use system(char*) procedure instead of execvp.

Otherwise, if you need to do the pipes yourself, you may want to parse the string with '|' special characters and use pipe(), fork() and I/O redirection. Like here How to run a command using pipe?