Done with Unix Single Piping but confused with Multipiping

[EliteLegend]

I'm trying my hand at implementing a small shell. I'm actually stuck at piping... I am able to handle a single pipe but how do I solve the problem of multiple pipes? I know it can be solved using recursion or looping(I'd love to hear more about this) but some pseudo algorithm will be excellent... I'm currently doing something like a parent creates two children and the first one executes one command and pipes it onto the second child which displays the output...

And when I used valgrind, to my surprise I found 15 memory leaks from the piping function that I wrote and I don't understand what could've gone wrong... My pseudo code looks something like this:

Code: Select all

	
	int fd[2]; /* provide file descriptor pointer array for pipe */
	pid_t pid1, pid2; /* process ids for each child */
	/* create pipe and check for an error */
	/* apply fork and check for error */
	    /* processing for child */
	    close (fd[1]); /* close output end, leaving input open */
	    /* set standard input to pipe */
	    if (fd[0] != STDIN_FILENO)
	    {
		if (dup2(fd[0], STDIN_FILENO) != STDIN_FILENO)
		{
		    perror("dup2 error for standard input");
		    exit(1);
		}
		close(fd[0]);
	    }
	   execlp the second function
	    //First child finished
	
	else
	{
	    /* processing for parent */
	    /* spawn second process */
	   /* apply fork again for second child*/
	  /* processing for child */
		
		close (fd[0]);
		/* set standard output to pipe */
		if (fd[1] != STDOUT_FILENO)
		{
		    if (dup2(fd[1], STDOUT_FILENO) != STDOUT_FILENO)
		    {
			perror("dup2 error for standard output");
			exit(1);
		    }
		    close(fd[1]); /* not needed after dup2 */
		}
		
		execlp the first function
		/* print to the pipe, now standard output */
	    }
	    else
	    {
		/* processing continues for parent */
		close(fd[0]);
		close(fd[1]);
		waitpid (pid1, NULL, 0); /* wait for first child to finish */
		waitpid (pid2, NULL, 0); /* wait for second child to finish */
	   }
	}

Am I doing something wrong? And could someone give me some tips on how to go about this issue? I've written my parser in such a way that it gives me something like:

argv[0] = ls
argv[1] = -la
argv[2] = |
argv[3] = wc
argv[4] = |
argv[5] = wc

for an input like "ls -la | wc | wc"

I am able to get it to work for "ls -la | wc" but the next thing is what is puzzling me... Is it possible using the architecture that I've written?

[JamesM]

Hi,

The way I would suggest is to recurse.

I would have a node structure. A 'command' is a node. So, nodes could be 'ls -la', 'wc', and 'wc'.

Each node would have two pointers: stdout and stdin. If these are NULL they would default to /dev/tty (so if you're not piping anything it just gets sent to the screen). They would also have a file handle as stdin.

Then, you could write a recursive function:

Code: Select all

void exec_cmd(node_t cmd)
{
  int pid = fork();
  if (pid == 0)
  {
    // child.
    if (cmd.stdout != NULL)
    {
      pipe...
      dup2(...);
      cmd.stdout.stdin = out_end_of_pipe;
    }
    execve
  }

  waitpid(pid);
  if (cmd.stdout)
  {
    exec_cmd(cmd.stdout);
  }
}

If I was doing it myself I'd probably have file descriptors for stdout,stderr,stdin and a 'pipe pointer' in the node structure, so I could easily redirect output to a file.

I think a node tree is the way forward - treating the command as bare text is going to get complicated very quickly!

Hope this helps a little bit,

JamesM

[EliteLegend]

Looks really interesting... Inorder to apply this I think I need to revise my parser properly... I'll be doing that... In the meanwhile, I have a few silly doubts:

Is this the way you want me to define the structure?

Code: Select all

typedef struct node_t {
		int stdin;
		int stdout;
		int argc;
		char *argv[BUFSIZE];
	} node_t;

and argc will contain something like "ls -la" and so on...

The command's stdin will be set if it is the last command in the chain and stdout will be set if its the first command. Am I right? And what would I be setting for intermediate commands or will recursion take care of that?

And what does this do?

Code: Select all

cmd.stdout.stdin = out_end_of_pipe; 

Does it set both stdout and stdin?

Sorry for the trouble... Its just that I'm not that good at recursion and hope to clear a few things before processing...

[JamesM]

Hi,

First of all as far as your parser is concerned I would look at flex/bison. Flex is a lexer (creates a token stream from a string source) and bison is a parser generator (GCC used to use flex/bison for it's front end). They're dead easy to use and simplify parsing a lot.

Is this the way you want me to define the structure?

Code: Select all

typedef struct node_t { 
      int stdin; 
      int stdout; 
      int argc; 
      char *argv[BUFSIZE]; 
   } node_t;

and argc will contain something like "ls -la" and so on...

Yes, looks about right.

The command's stdin will be set if it is the last command in the chain and stdout will be set if its the first command. Am I right? And what would I be setting for intermediate commands or will recursion take care of that?

The way I was suggesting is - for the first command in the chain, stdin == /dev/tty. Then, whenever you are about to execve a process that will have it's output piped to another process, you look up that other node (cmd.stdout), and set it's stdin to the readable end of a pipe. Set the current process' stdout to the writeable end of a pipe. Done.

Take the ideas I'm giving you with a pinch of salt: The ideas themselves are sound but the node structure could do with a lot of work. I'm working now on a solution to post on the wiki (people have been asking for a page about shells).

Cheers! (and feel free to bombard me with questions, I dont mind )

[EliteLegend]

Well, I'll give it a shot... For some reason pipes have been the only things that have been haunting me for a long time... Though I'm still not into recursion, I'd love to see some sample that utilizes it efficiently... I can sense how it works through recursion (its just great) but getting it to work is like a nightmare to me...