forked processes with pipes having different output

Viewed 133

I have some code practicing forking processes(mocking the (|) in command line). However, the output is not the same every time. For example, for the input of ./pipe ls cat wc, it should be the same as ls | cat | wc . However, sometimes my code will output

Child with PID 12126 exited with status 0x0.

Child with PID 12127 exited with status 0x0.

      7       7      52

Child with PID 12128 exited with status 0x0.

But sometimes it will also ouutput:

Makefile

pipe

#pipe.c#

pipe.c

pipe.o

README.md

test

Child with PID 12138 exited with status 0x0.

Child with PID 12139 exited with status 0x0.

      0       0       0

Child with PID 12140 exited with status 0x0.

Which the first output is the correct one(compared with ls | cat | wc). I figured that by the second output, the output of the piping of the programs ls and cat is not being processed by wc. I am wondering what went wrong with my programs because seems like I set up the piping correctly - first program will take the input from stdin and output to the write end of the pipe and the last program will take the input from the read end of the pipe and output to stdout. Any inputs are appreciated.

Code(./pipe):

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

int main(int argc, char *argv[])
{
  int fd[2];
  if (pipe(fd) == -1) {
    int err = errno;
    perror("pipe");
    return err;
  }
  pid_t pid[argc-1];
  int n = argc;
  if(argc <= 1){
    return EINVAL;
  }
  for (int i = 1; i < argc; i++){
    if ((pid[i] = fork()) == -1){
      int err = errno;
      perror("fork");
      return err;
    }else if(pid[i] == 0){

      //open(fd[1]);
      if(i == 1){
        dup2(fd[1],STDOUT_FILENO);
        if (execlp(argv[i], argv[i], NULL) == -1) {
          printf("failed to search for the provided executed program. \n");
          return errno;
        }
        
        
      }else if(i == argc-1){
        dup2(fd[0],STDIN_FILENO);
        if (execlp(argv[i], argv[i], NULL) == -1) {
          printf("failed to search for the provided executed program. \n");
          return errno;
        }
      }else{
          dup2(fd[0],STDIN_FILENO);
          dup2(fd[1],STDOUT_FILENO);
          if (execlp(argv[i], argv[i], NULL) == -1) {
            printf("failed to search for the provided executed program. \n");
            return errno;
          }
      }
      
    }
     close(fd[1]);
  }
  
  int wstatus;
  pid_t pids;
  while (n > 1) {
    pids = wait(&wstatus);
    printf("Child with PID %ld exited with status 0x%x.\n", (long)pids, wstatus);
    --n;
  }

}
2 Answers

You are accessing the Variable Length Array pid out of bounds so your program has undefined behavior.

You could solve it by making the array one element larger:

pid_t pid[argc]; /* not argc - 1 */

Another problem is that you need a new pipe between each pair of commands. In total, one pipe less than the number of commands. That is, zero pipes for one command.

stdin - ls - pipe - cat - pipe - wc - stdout   // 3 commands, 2 pipes

Here's one idea where I create a pipe in (almost) every iteration of the loop and save the input side of the pipe for the next iteration. I don't bother saving the process id:s at all and I haven't added error checking for dup2. You should however check that it actually succeeds.

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

int exec(const char* prog) {
    if(execlp(prog, prog, NULL) == -1) {
        perror("execlp");
    }
    return 1;
}

int main(int argc, char* argv[]) {
    pid_t pid;

    int in; // for saving the input side of the pipe between iterations

    for(int i = 1; i < argc; ++i) {
        int fd[2];

        // must have at least 2 commands to create a pipe
        // and we need one pipe less than the number of commands
        if(argc > 1 && i < argc - 1) pipe(fd);

        if((pid = fork()) == -1) {
            perror("fork");
            return 1;

        } else if(pid == 0) {
            if(i == 1) {       // first command
                if(argc > 1) { // must have more than one command to dup
                    dup2(fd[1], STDOUT_FILENO);
                    close(fd[1]);
                    close(fd[0]);
                }

            } else if(i == argc - 1) { // last command
                dup2(in, STDIN_FILENO);
                close(in);

            } else { // middle commands
                dup2(in, STDIN_FILENO);
                close(in);
                dup2(fd[1], STDOUT_FILENO);
                close(fd[1]);
                close(fd[0]);
            }

            return exec(argv[i]);
        }

        // parent
        if(i > 1) close(in);  // close old in
        in = fd[0]; // save the stdin side of the pipe for next loop iteration
        close(fd[1]); // no use for the stdout end
    }

    int wstatus;
    while((pid = wait(&wstatus)) != -1) {
        printf("Child with PID %ld exited with status 0x%x.\n", (long)pid,
               wstatus); 
    }
}

You need more pipes, since there's only one in your current code. If ls and cat are both writing into that pipe, and cat and wc are both reading from that pipe, you cannot synchronize which data cat reads verses the data that wc reads. That is, wc might read data directly from ls, or from cat. And cat might read data from itself. (Which seems to be what sometimes happens). The order in which the processes read data from the pipe will depend on the order in which they are scheduled to run.

You need more pipes. (One fewer than the total number of processes.)

Related