0

I am trying to implement multi pipe in C, to run multiple commands like a shell. I have made a linked list (called t_launch in my code) which look like that if you type "ls | grep src | wc" :

wc -- PIPE -- grep src -- PIPE -- ls

Every PIPE node contain an int tab[2] from the pipe() function (of course, there have been one pipe() call for each PIPE node)

Now i am trying to execute these commands :

int     execute_launch_list(t_shell *shell, t_launch *launchs)
{
   pid_t pid;
   int   status;
   int   firstpid;

   firstpid = 0;
   while (launchs != NULL)
   {
      if ((pid = fork()) == -1)
         return (my_error("Unable to fork\n"));
      if (pid == 0)
      {
         if (launchs->prev != NULL)
         {
            close(1);
            dup2(launchs->prev->pipefd[1], 1);
            close(launchs->prev->pipefd[0]);
         }
         if (launchs->next != NULL)
         {
            close(0);
            dup2(launchs->next->pipefd[0], 0);
            close(launchs->next->pipefd[1]);
         }
        execve(launchs->cmdpath, launchs->words, shell->environ);
      }
      else if (firstpid == 0)
        firstpid = pid;
      launchs = launchs->next == NULL ? launchs->next : launchs->next->next;
   }
   waitpid(firstpid, &status, 0);
   return (SUCCESS);
}

But that doesn't work : it looks like commands dont stop reading. For example if i type "ls | grep src, "src" will be print from the grep command, but the grep continue reading and never stop. If i type "ls | grep src | wc", nothing is printed. What's wrong with my code ? Thank you.

Algo
  • 65
  • 6
  • Could you explain `launchs = launchs->next == NULL ? launchs->next : launchs->next->next;` please? Am I missing sonethin or do you skip every second child process? – Ingo Leonhardt Jul 11 '13 at 16:34
  • I skip the node "PIPE" : The linked list looks like "grep -- PIPE -- ls" so after i did the "grep stuff", i go to "ls" so it is ->next->next (a single next would bring me on a pipe node) – Algo Jul 11 '13 at 16:49
  • thanks I understand now. I guess rici is right: a client should get `stdin` from `launch->prev` and redirect `stdout` to `launchs->next`. ... oops, his comment ist deleted, but nevertheless ... – Ingo Leonhardt Jul 11 '13 at 17:03
  • @IngoLeonhardt: I deleted it because I saw that the order of the list is right-to-left, so next and prev are correct. – rici Jul 11 '13 at 17:17
  • @rici I'm now cured of my blindness, btw. good answer, algo, forget my last comment and look at rici's answer – Ingo Leonhardt Jul 11 '13 at 17:24

1 Answers1

1

If I understand your code correctly, you first call pipe in the shell process for every PIPE. You then proceed to fork each process.

While you do close the unused end of each of the child's pipes in the child process, this procedure suffers from two problems:

  1. Every child has every pipe, and doesn't close the ones which don't belong to it

  2. The parent (shell) process has all the pipes open.

Consequently, all the pipes are open, and the children don't get EOFs.

By the way, you need to wait() for all the children, not just the last one. Consider the case where the first child does some long computation after closing stdout, but remember that any computation or side-effect after stdout is closed, even a short one, could be sequenced after the sink process terminates since multiprocessing is essentially non-deterministic.

rici
  • 234,347
  • 28
  • 237
  • 341
  • Thank you all, my implementation was really bad. I did a new code using recursion and it works perfectly. Thanks again. – Algo Jul 11 '13 at 18:59