Piping between several processes in C

Question

I'm writing a shell in C and am trying to implement multiple pipes. I've done this by creating a two dimensional array with pipes and using a separate pipe everytime. All commands in between pipes are separated by a parsing function and put into a struct. Every command line in-between pipes gets it's own process. And for all commands in the middle I'm trying to read from the previous process and write to the next one. Somewhere here the problem starts. It works fine for one pipe, however when I trying more than one pipe I don't get any output and the program gets stuck. In GDB I get a failed message from the execvp after forking the second process. What can this be due to?


int create_pipe(int* fd)
{
  int pipe_id = pipe(fd); 
  if (pipe_id == -1)
  {
    return -1; 

  }
  return 0; 
}

void write_pipe(int* fd)
{
  close(fd[READ]);
  if ((dup2(fd[WRITE], STDOUT_FILENO)) < -1) 
  {
    fork_error(); 
  }
  close(fd[WRITE]);
 
}

void read_pipe(int *fd)
{
  close(fd[WRITE]);
  if (dup2(fd[READ], STDIN_FILENO) < 0)
  {
    fork_error();
  }
  close(fd[READ]);
  
}

void need_to_pipe (int i, int (*fd)[2])
{

  if (commands[i].pos == first)
  { 
    write_pipe(fd[i * 2]);
  }
  else if (commands[i].pos == last)
  {
    read_pipe(fd[(i-1) *2]); 
  }
  else //if (commands[i].pos == middle) 
  {
    
      dup2(fd[(i-1)*2][READ], STDIN_FILENO);
      close(fd[(i-1)*2][READ]);
      close(fd[(i-1)*2][WRITE]);
      //close(fd[(i)*2][READ]);
      //close(fd[(i)*2][WRITE]);
      close(fd[(i)*2][READ]);
      dup2(fd[i*2][WRITE], STDOUT_FILENO);
      close(fd[(i)*2][WRITE]); 
  }
  
  
}

void fork_cmd(int i, int (*fd)[2]) {

  pid_t pid; 
  switch (pid = fork()) {
    case -1:
      fork_error();
    case 0:
      if (!(commands[i].pos == single))
      {
        need_to_pipe(i, fd);
      }
      if (execvp(commands[i].argv[0], commands[i].argv)<0)
      {

        exit(EXIT_FAILURE);
      }
  }
}

void fork_cmds(int n, int (*fd)[2]) 
{
  for (int i = 0; i < n; i++) 
  {
    fork_cmd(i, fd);
  }
}


void wait_once ()
{
  wait(NULL);
}

void wait_for_all_cmds(int n) 
{
  for (int i = 0; i < n; i++)
  {
      wait_once();
     //wait for number of child processes. 
  }

}


int main() {
  int n;               
  size_t size = 256;   
  char line[size];        

  while(true) {
       

    get_line(line, size);

    n = parse_cmds(line, commands);

    int fd[(n-1)][2]; 
    for(int i =0;i<n-1;i++)
    {
      int pipe_id = pipe(fd[i*2]); 

      if (pipe_id == -1)
      {
        return -1; 

      }
    }

    fork_cmds(n, fd);

    for(int i =0;i<n-1;i++)
    {
      int *fdclose= fd[i*2]; 

      close (fdclose[READ]);
      close (fdclose[WRITE]);
    }
    
    wait_for_all_cmds(n);
    
  }

  exit(EXIT_SUCCESS);
}

Aside: ```printf(" >>> ");``` ---> Either add a `\n`, or flush `stdout`. You may not see a prompt elsewise. — Harith, Feb 13 '23 at 01:21
Your function `int create_pipe(int* fd) { int pipe_id = pipe(fd); if (pipe_id == -1) { return -1; } return 0; }` is a verbose way of writing `int create_pipe(int* fd) { return pipe(fd); }` — and then you really don't need the `create_pipe()` function after all (just call `pipe()` directly. There's no benefit to the extra 5 lines of code in the body of `create_pipe()`. — Jonathan Leffler, Feb 13 '23 at 01:35

Craig Estey · Answer 1 · 2023-05-03T18:36:40.853

You [probably] have too many processes keeping pipe ends open (that do not belong to the given child) because your loop opens all pipes before any forking.

This places an undue burden on each child because it has to close many pipe ends to prevent it from holding open a pipe end, preventing other children from seeing an EOF on their input pipes.

To see this, for debug purposes in your present code, the child could do (just before the exec* call) (e.g.):

pid_t pid = getpid();

fprintf(stderr, "child: %d\n", pid);
fflush(stderr);

char cmd[100];
sprintf(cmd,"ls -l /proc/%d/fd 1>&2",pid);

system(cmd);

Each child should only have three open streams on stdin, stdout, and stderr (e.g. 0, 1, 2).

I think you'd find that there are many extraneous/detrimental streams open on the various children.

You only need two pipe arrays (e.g.): int pipeinp[2]; int pipeout[2]; Initially, pipeinp is all -1.

Roughly ...

Parent should do a single pipe call at the top of fork_cmd [before the fork] to pipeout.

The child dups (and closes) the read end of pipeinp [if not -1] to stdin.

Child dups/closes the write end of pipeout to stdout.

It closes the read end of pipeout.

After that, the parent should copy pipeout to pipeinp and close the write end of pipeinp

This should be repeated for all pipe stages.

No pipe to pipeout should be done for the last command. And, the [last] child should not change stdout.

For a working example, see my answer: fd leak, custom Shell

Piping between several processes in C

1 Answers1

Linked