Create a shell

>> but not pipe and dup2...

When you refer back to the explanation of pipes I gave in your other question :

        https://www.experts-exchange.com/questions/26902191/exec-fail.html#35189801

(as well as the posts after that), is there anything that is still unclear about them ?


>> because there're should be another process(this should carry the 2nd command) under my first child process, am i right?

If you want to create a pipe between two processes, then you need two processes indeed :)

yes this 1 i get it, i made a slight changes in my code... when it is in the 1st command (temp(0) in this case, here i should have dup2(fds[1],1); close(fds[0]);      // Reasign stdout to fds[1] end of pipe and then this pipe will link it to the stdin of the 2nd command as in dup2(fds[0],0); close fds[1];
i'm not sure what should i do in the first else if((pid = fork()) == 0)?
and how should i link the 2nd command with the 3rd one?

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

int main()
{
	int i = 0;
	int k = 0;
	int j;
	int nos;
	int no = 1;
	int pos, s;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char* temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any
	char* argv[80];		// an array to store the arguments

	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		command = strtok(buf,"|");		// tokenize the command line by |
		printf("separate by |: \n");
		while(command != NULL)
		{
			nos = strspn (command, space); 	// return the number of spaces or tab before the command
			for (s = 0; s < nos; s++)
				++command;		// increment the pointer of command to remove the spaces or tabs, if any
			if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
				command[pos] = '\0';	// replace it with a null, if any			
			for (;;)				// make a loop
			{
				int n = strlen (command) - 1;	// last character position in command
				if (command[n] == ' ')		// if there's a space 
					command[n] = 0;		// makes the string shorter
				else 
					break;			// out of the loop, if any non-white space is found
			}			

			printf("%s\n",command);
			temp [i] = command;		// store each commands into the array
			command = strtok(NULL,"|");
			i++;				// increment array
		}

		printf("Command and its process id: \n");

		for(j = 0; j < i; j++)			// fork each command
		{
			int fds[2];			// file descriptor
			pipe(fds);
			pid = fork();
			if (pid == 0)			// child
			{
				argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				
				dup2(fds[1],1);		// Reasign stdout to fds[1] end of pipe
				close (fds[0]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command		
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);
			}
			else if ((pid = fork())== 0)
				argument = strtok((char *)temp[j+1]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				dup2(fds[0],0);		// Reassign stdin to fds[0] end of pipe
				close(fds[1]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);		
			else if (pid < 0) 		// error
			{
				printf("fork failed");
				exit(1);
			}
			else				// parent
			{
				waitpid(pid, NULL, 0);
			}
			
		}

        }
	return 0;
}

Select allOpen in new window

okay I'm not sure whether i get it correct or not but it seems to be like this:

if (pid == 0)			// child
			{
				argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				close (fds[0]);         // close the read end of the pipe
				dup2(fds[1],1);		// Reasign stdout to fds[1] end of pipe
				close (fds[1]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command		
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);
			}
			else if ((pid = fork())== 0)
				argument = strtok((char *)temp[j+1]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				close(fds[1]);           // close the write end of the pipe
				dup2(fds[0],0);		// Reassign stdin to fds[0] end of pipe
				close(fds[0]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);		

Select allOpen in new window

Apart from the missing {}'s around the else if block, that looks ok at first sight.

yeah i knew that mistake after posting it, so i corrected it immediately, anyway thx for reminding:)
but i would like to ask some question because currently i'm using temp (j+1) in the else if, is that fine if i put like this?
what should i do next because this look somehow only for 2 commands and now i think i need to work on those middle commands that have fds[0] and fds[1] open for use, am i right?

>> but i would like to ask some question because currently i'm using temp (j+1) in the else if, is that fine if i put like this?

If that is what you intended to do, then it's fine. What makes you think it isn't ?


>> what should i do next because this look somehow only for 2 commands and now i think i need to work on those middle commands that have fds[0] and fds[1] open for use, am i right?

What do you mean by "middle commands" ?

i'm not sure why, although i put that myself but it seems weird to me dunno why, that'y i was asking for double confirmation:)

hmmm something like this
cmd1 | cmd2 | cmd3 | cmd4
cmd 1 output to cmd2 input and then run again the for loop (j++) now i should be in the 3rd command so shouldn't the 3rd command get the input from (cmd2 output) and output to (cmd4 input)?
hmm do you get what i meant?

>> cmd1 | cmd2 | cmd3 | cmd4

In this case, you'd need to create three distinct pipes, and 4 distinct processes.

hmm can you explain in more details or maybe example on how can i write so that it goes automatically (i know my j is the one that can runs automatically for the number of commands found on the user command line but how should i write for pipe in this case)?

In the same way that you created a pipe between the first two processes above, you can create a second pipe between the second process and a third process. And similarly, another pipe between the third and fourth process.

so should i use smt a for loop again to run for arbitrary times but this time we'll be doing for both fds[0] and fds[1] ... i'm abit confused here how can i write so that it knows the 1st and the last command are the ones that i've previously wrote and another one for those in between if there're more than 2 commands?

You can write a loop, and create a new process and a new pipe for every iteration, yes (as long as you create the first process before the loop).

sorry that i was away for few days to prepare for my exams:) hahah ok now so back to this assignment...
hmm i made some changes but i'm not sure whether am i writing it correctly, because i was abit confused how should i arrange all these commands with pipes and then fork it...
i edited it as the first command will be executing before the loop and the rest (from the 2nd - last command in the for loop, j = 1) but it seems somewhere weird to me in the for loop and i'm not sure what should i do...

printf("Commands with pipes: \n");

		int fds[2];			// file descriptor
		pipe(fds);
		pid = fork();
		if (pid == 0)			// child
		{
			argument = strtok((char *)temp[0]," \t\n");	// tokenize the command into arguments
			while (argument != NULL)
			{
				argv[k] = argument;			
				// store each arguments into the array
				argument = strtok(NULL," \t\n");
				k++;
			}
			close (fds[0]);		// close the read end of the pipe
			dup2(fds[1],1);		// Reasign stdout to fds[1] end of pipe
			close (fds[1]);		// Not going to write in this child process, so we can close this end of the pipe
			execvp(argv[0],argv);	// execute the given command		
			write(1,"exec fail\n",10);	// print this message if fails to execvp		
			exit(1);
		}
		for(j = 1; j < i; j++)			// fork each command
		{
			pipe(fds);
			pid = fork();
			if (pid == 0)			// child
			{
				argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				dup2(fds[0],0);		// Reassign stdin to fds[0] end of pipe
				close(fds[0]);		// Not going to write in this child process, so we can close this end of 
				dup2(fds[1],1);		// Reasign stdout to fds[1] end of pipe
				close (fds[1]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command		
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);
			}
			else if ((pid = fork())== 0)
			{
				argument = strtok((char *)temp[j+1]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
				close(fds[1]);		// close the write end of the pipe
				dup2(fds[0],0);		// Reassign stdin to fds[0] end of pipe
				close(fds[0]);		// Not going to write in this child process, so we can close this end of the pipe
				execvp(argv[0],argv);	// execute the given command
				write(1,"exec fail\n",10);	// print this message if fails to execvp		
				exit(1);
			}		
			else if (pid < 0) 		// error
			{
				printf("fork failed");
				exit(1);
			}
			else				// parent
			{
				waitpid(pid, NULL, 0);
			}
			
		}

        }
	return 0;
}

Select allOpen in new window

You only have one pipe :

>>             int fds[2];                  // file descriptor

You need more than one to be able to make this work ;)

i did some changes, but i was so confuse while writing these especially the pipes, i'm not sure whether correct or not as i introduced two pipes, int fdl[2], fdr[2];
here is the code

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

int main()
{
	int i = 0;
	int k = 0;
	int j;
	int nos;
	int no = 1;
	int pos, s;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char* temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any
	char* argv[80];		// an array to store the arguments
	int status;

	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		command = strtok(buf,"|");		// tokenize the command line by |
		printf("separate by |: \n");
		while(command != NULL)
		{
			nos = strspn (command, space); 	// return the number of spaces or tab before the command
			for (s = 0; s < nos; s++)
				++command;		// increment the pointer of command to remove the spaces or tabs, if any
			if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
				command[pos] = '\0';	// replace it with a null, if any			
			for (;;)				// make a loop
			{
				int n = strlen (command) - 1;	// last character position in command
				if (command[n] == ' ')		// if there's a space 
					command[n] = 0;		// makes the string shorter
				else 
					break;			// out of the loop, if any non-white space is found
			}			

			printf("%s\n",command);
			temp [i] = command;		// store each commands into the array
			command = strtok(NULL,"|");
			i++;				// increment array
		}

		printf("Commands with pipes: \n");

		int fdl[2];			// file descriptor
		int fdr[2];			// file descriptor
		pid = fork();
		if (pid == 0)			// first child (first command from left)
		{
			pipe (fdl);
			pid = fork;
			if (pid > 0)		// first command is new parent
			{			// first and second command share a pipe
				argument = strtok((char *)temp[0]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
		
				close (fdl[0]);			// close the read end of the pipe
				fdl[1] = dup2(fds[1],1);	// Reasign stdout to fds[1] end of pipe
				execvp(argv[0],argv);		// execute the given command		
				write(1,"exec first command fail\n",25);	// print this message if fails to execvp		
				exit(1);
			}	// exit the first command
		
			for(j = 1; j <= i; j++)			// fork each middle command
			{
				if (pid == 0)			// child
				{
					fdr[0] = fdl[0];
					fdr[0] = dup2(fds[0],0);// Reassign stdin to fds[0] end of pipe
					fdr[1] = fdl[1];
					close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					pipe(fdl);
					pid = fork();	// second command hatches third or following commands
					if (pid > 0)	// still in the middle commands
					{
						argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
						while (argument != NULL)
						{
							argv[k] = argument;			
							// store each arguments into the array
							argument = strtok(NULL," \t\n");
							k++;
						}
						close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
						fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
						execvp(argv[0],argv);		// execute the given command		
						write(1,"exec second command or later on fail\n",10);	// print this message if fails to execvp		
						exit(1);
					}
					while( j = i && pid == 0)		// in the last command 
					{
						argument = strtok((char *)temp[i]," \t\n");	// tokenize the command into arguments
						while (argument != NULL)
						{
							argv[k] = argument;			
							// store each arguments into the array
							argument = strtok(NULL," \t\n");
							k++;
						}
						fdr[0] = fdl[0];
						fdr[1] = fdl[1];
						close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
						fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
						execvp(argv[0],argv);		// execute the given command
						write(1,"exec last command fail\n",10);	// print this message if fails to execvp		
						exit(1);			
					}	// exit the last commant
				}	// within the middle command loop
			}
        	}
		waitpid (pid, &Status, 0);
	}	// end while loop	
	return 0;
}

Select allOpen in new window

Before you use a pipe, you have to initialize it (ie. use the pipe function).

oops sorry is it like this.... hmm did i make any mistakes in those piping? i was confused but i'm trying my best to put accordingly (as sometimes i mixed up the 0 and 1)

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

int main()
{
	int i = 0;
	int k = 0;
	int j;
	int nos;
	int no = 1;
	int pos, s;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char* temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any
	char* argv[80];		// an array to store the arguments
	int status;

	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		command = strtok(buf,"|");		// tokenize the command line by |
		printf("separate by |: \n");
		while(command != NULL)
		{
			nos = strspn (command, space); 	// return the number of spaces or tab before the command
			for (s = 0; s < nos; s++)
				++command;		// increment the pointer of command to remove the spaces or tabs, if any
			if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
				command[pos] = '\0';	// replace it with a null, if any			
			for (;;)				// make a loop
			{
				int n = strlen (command) - 1;	// last character position in command
				if (command[n] == ' ')		// if there's a space 
					command[n] = 0;		// makes the string shorter
				else 
					break;			// out of the loop, if any non-white space is found
			}			

			printf("%s\n",command);
			temp [i] = command;		// store each commands into the array
			command = strtok(NULL,"|");
			i++;				// increment array
		}

		printf("Commands with pipes: \n");

		int fdl[2];			// file descriptor
		int fdr[2];			// file descriptor
		pid = fork();
		if (pid == 0)			// first child (first command from left)
		{
			pipe (fdl);
			pid = fork;
			if (pid > 0)		// first command is new parent
			{			// first and second command share a pipe
				argument = strtok((char *)temp[0]," \t\n");	// tokenize the command into arguments
				while (argument != NULL)
				{
					argv[k] = argument;			
					// store each arguments into the array
					argument = strtok(NULL," \t\n");
					k++;
				}
		
				close (fdl[0]);			// close the read end of the pipe
				fdl[1] = dup2(fds[1],1);	// Reasign stdout to fds[1] end of pipe
				execvp(argv[0],argv);		// execute the given command		
				write(1,"exec first command fail\n",25);	// print this message if fails to execvp		
				exit(1);
			}	// exit the first command
		
			for(j = 1; j <= i; j++)			// fork each middle command
			{
				if (pid == 0)			// child
				{
					pipe(fdr);
					fdr[0] = fdl[0];
					fdr[0] = dup2(fds[0],0);// Reassign stdin to fds[0] end of pipe
					fdr[1] = fdl[1];
					close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					pipe(fdl);
					pid = fork();	// second command hatches third or following commands
					if (pid > 0)	// still in the middle commands
					{
						argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
						while (argument != NULL)
						{
							argv[k] = argument;			
							// store each arguments into the array
							argument = strtok(NULL," \t\n");
							k++;
						}
						close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
						fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
						execvp(argv[0],argv);		// execute the given command		
						write(1,"exec second command or later on fail\n",40);	// print this message if fails to execvp		
						exit(1);
					}
					while( j = i && pid == 0)		// in the last command 
					{
						argument = strtok((char *)temp[i]," \t\n");	// tokenize the command into arguments
						while (argument != NULL)
						{
							argv[k] = argument;			
							// store each arguments into the array
							argument = strtok(NULL," \t\n");
							k++;
						}
						fdr[0] = fdl[0];
						fdr[1] = fdl[1];
						close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
						fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
						execvp(argv[0],argv);		// execute the given command
						write(1,"exec last command fail\n",25);	// print this message if fails to execvp		
						exit(1);			
					}	// exit the last commant
				}	// within the middle command loop
			}
        	}
		waitpid (pid, &Status, 0);
	}	// end while loop	
	return 0;
}

Select allOpen in new window

Have you noticed that you have a lot of very similar code repeated over and over again ?

Have you also noticed that your main is getting rather big, and difficult to find your way through ?

It's a good idea to write some functions that perform specific operations, so you can split up the code (modularize it) in more easy to digust small chunks.
Having functions also allows you to call the same function multiple times, using parameters to specify differences.

hmm i've put it into 2 funcs, one for command and one for arguments but i'm not sure whether i've wrote it correctly especially the parameters? and other than putting it into func is there anything that i've did it wrongly?

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

void commands(int i)
{
	int nos, pos, s;
	char *command;
        char buf[80];   	// command line
        char* temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any
	char* argv[80];		// an array to store the arguments

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}
	return i;
}

void arguments(int j)
{
	int k = 0;
	char *argument;	
	char* argv[80];		// an array to store the arguments
        char* temp[80]={0};	// an array to store the commands
	argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
	while (argument != NULL)
	{
		argv[k] = argument;			
		// store each arguments into the array
		argument = strtok(NULL," \t\n");
		k++;
	}
	j++;
	return j;
}

int main()
{
	int count = 0;
	int size = 0;
	int i, j;
	char* argv[80];		// an array to store the arguments
	int status;

	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		commands(count);

		printf("Commands with pipes: \n");

		int fdl[2];			// file descriptor
		int fdr[2];			// file descriptor
		pid = fork();
		if (pid == 0)			// first child (first command from left)
		{
			pipe (fdl);
			pid = fork;
			if (pid > 0)		// first command is new parent
			{			// first and second command share a pipe
				arguments(size);		
				close (fdl[0]);			// close the read end of the pipe
				fdl[1] = dup2(fds[1],1);	// Reasign stdout to fds[1] end of pipe
				execvp(argv[0],argv);		// execute the given command		
				write(1,"exec first command fail\n",25);	// print this message if fails to execvp		
				exit(1);
			}	// exit the first command
		
			for(j = 1; j <= i; j++)			// fork each middle command
			{
				if (pid == 0)			// child
				{
					pipe(fdr);
					fdr[0] = fdl[0];
					fdr[0] = dup2(fds[0],0);// Reassign stdin to fds[0] end of pipe
					fdr[1] = fdl[1];
					close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					pipe(fdl);
					pid = fork();	// second command hatches third or following commands
					if (pid > 0)	// still in the middle commands
					{
						arguments(size);		
						close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
						fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
						execvp(argv[0],argv);		// execute the given command		
						write(1,"exec second command or later on fail\n",40);	// print this message if fails to execvp		
						exit(1);
					}
					while( j = i && pid == 0)		// in the last command 
					{
						arguments(size);		
						fdr[0] = fdl[0];
						fdr[1] = fdl[1];
						close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
						fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
						execvp(argv[0],argv);		// execute the given command
						write(1,"exec last command fail\n",25);	// print this message if fails to execvp		
						exit(1);			
					}	// exit the last commant
				}	// within the middle command loop
			}
        	}
		waitpid (pid, &Status, 0);
	}	// end while loop	
	return 0;
}

Select allOpen in new window

First validate if the code is behaving the way it should. And if not, what is going wrong ?

i know something is missing because when i did the func for command and argument, the temp[] and argv[] are not passing in the main....so i made some changes... but i'm not sure whether the parameters are correct or not to put in this way...if not what should i do for the parameters in the func... thank you.

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

void commands(char *temp[], int i)
{
	int nos, pos, s;
	char *command;
        char buf[80];   	// command line
        char* temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}
	return i;
}

void arguments(char *temp[], char *argv[], int j)
{
	int k = 0;
	char *argument;	
	char* argv[80];		// an array to store the arguments
        char* temp[80]={0};	// an array to store the commands
	argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
	while (argument != NULL)
	{
		argv[k] = argument;			
		// store each arguments into the array
		argument = strtok(NULL," \t\n");
		k++;
	}
	j++;
	return j;
}

int main()
{
	int count = 0;
	int size = 0;
	int i, j;
	char* argv[80] = char *arg[80];		// an array to store the arguments
	char *temp[80] = {0};
	int status;

	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		commands(temp, count);
		printf("Commands with pipes: \n");

		int fdl[2];			// file descriptor
		int fdr[2];			// file descriptor
		pid = fork();
		if (pid == 0)			// first child (first command from left)
		{
			pipe (fdl);
			pid = fork;
			if (pid > 0)		// first command is new parent
			{			// first and second command share a pipe
				arguments(temp, arg, size);		
				close (fdl[0]);			// close the read end of the pipe
				fdl[1] = dup2(fds[1],1);	// Reasign stdout to fds[1] end of pipe
				execvp(argv[0],argv);		// execute the given command		
				write(1,"exec first command fail\n",25);	// print this message if fails to execvp		
				exit(1);
			}	// exit the first command
		
			for(j = 1; j <= i; j++)			// fork each middle command
			{
				if (pid == 0)			// child
				{
					pipe(fdr);
					fdr[0] = fdl[0];
					fdr[0] = dup2(fds[0],0);// Reassign stdin to fds[0] end of pipe
					fdr[1] = fdl[1];
					close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					pipe(fdl);
					pid = fork();	// second command hatches third or following commands
					if (pid > 0)	// still in the middle commands
					{
						arguments(temp, arg, size);		
						close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
						fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
						execvp(argv[0],argv);		// execute the given command		
						write(1,"exec second command or later on fail\n",40);	// print this message if fails to execvp		
						exit(1);
					}
					while( j = i && pid == 0)		// in the last command 
					{
						arguments(temp, arg, size);		
						fdr[0] = fdl[0];
						fdr[1] = fdl[1];
						close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
						fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
						execvp(argv[0],argv);		// execute the given command
						write(1,"exec last command fail\n",25);	// print this message if fails to execvp		
						exit(1);			
					}	// exit the last commant
				}	// within the middle command loop
			}
        	}
		waitpid (pid, &Status, 0);
	}	// end while loop	
	return 0;
}

Select allOpen in new window

You've grown your code by adding new stuff and more new stuff into the existing lump of code, up to the point where it became difficult to manage for you, and you had to ask for help.

You can avoid all that, by designing your code to be modular to begin with. And make sure to test each bit of code extensively, so you're sure that it works, before continuing.

I would recommend that you start with a clean slate, and think about how you want to organize the code to meet the required goals.

When programming, you spend most of your time thinking about what to write, and how to write it, and only very little time actually writing down the code. The more you think before writing, the less problems you're likely to have.

hmm but first did i do it correctly in this post http:#35361600 because that is before i started to split up the code into funcs? if is correct then i'll have to start from there and work on it... if not can you lead me step by step... or any materials/examples so that it is easier for me to understand...

What I was talking about was already true before you added the functions.

so that means the problem started when i introduced the two pipes fdl[2] and fdr[2]??
can you give me some steps that i can follow how to use 2 pipes to work together with the commands because i'm confused which 1 i should do first....

>> so that means the problem started when i introduced the two pipes fdl[2] and fdr[2]??

No, the problem started when you started writing this code.

With the knowledge you have now, you should be able to come up with a nice design, such that all needs are met, and the code will be easier to maintain.

huh from the start? because i thought i can just continued from my previous assignment that using fork and execvp and now i should add in the pipe func... shouldn't it be like that?
ok let's have some steps that i need to do in order for the shell to work for pipes with commands....
correct me if i'm wrong...

1. parse the commands line into commands by |
2. fork a process - the child will be the first command.
3. initialize a pipe
4. fork a process - the parent now is the first command and the child is the 2nd command
5. close read input and write output to 2nd command - execvp the first command
6. at the child (second command) initialize a pipe
7. fork a process -  the parent now is the 2nd command and the child is the 3rd command
8. read the input from 2nd command and write output to 3rd command - execvp the 2nd command
9. repeat 6-8 for middle commands
10. at the child (last command)
11. read the input and clost the write output - execvp the last command.

>> because i thought i can just continued from my previous assignment

You could. But the more code you add, the more complicated it will get to understand/maintain it.

If you don't think about modularizing your code from the start, it's a lot harder afterwards to clean the existing code up.

In the new code, the techniques and such that you used in the existing code can be re-used. However, what should be different, is the way it's organized in the source code file.

You could for example have a function that does the parsing, and a function for launching a new process. Think about how you could keep each function (including main) as small as possible. If it doesn't fit on a screen, it's almost definitely too long, and needs to be split up.

hmmm i've spread out the code into 3 different funcs, but because due to the machines are all down i couldn't test anything....but here's the code... can you have a look at it... if there's anything wrong?

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

void ParseInput(char *buf, char *command, int count)
{
	int i = 0;
	int nos, s, pos;
	char *temp[80]={0};	// an array to store the commands
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(*buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	count = i;
}

void ParseCommand(char *temp, char *argument)
{
	int k = 0;
	int j = 0;
	argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
	while (argument != NULL)
	{
		argv[k] = argument;			
		// store each arguments into the array
		argument = strtok(NULL," \t\n");
		k++;
	}
}

void execute(char *argv)
{
	pid_t pid;
	int status;
	pid = fork();
	if (pid == 0)			// child
	{
		ParseCommand(temp, argument);
		execvp(*argv[0],argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, command, count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

The general idea of your main is good, but a few remarks :

1) when passing data back to the calling function via a parameter, you have to pass that parameter by pointer - otherwise the calling function won't see any change.

2) you are often using variables that haven't been defined.

Good start !

>>when passing data back to the calling function via a parameter, you have to pass that parameter by pointer - otherwise the calling function won't see any change.

hmm which one do you meant... is it the "command" that i used in void ParseInput(char *buf, char *command, int count)

>>you are often using variables that haven't been defined.

ahh did u meant the temp(i) from void ParseInput seems to be not passing to the ParseCommand...so should i add in like this void ParseInput(char *buf, char *command, char *temp, int count)?
if not then which one?

>> hmm which one do you meant...

All of the functions where you try to pass data back to the calling function, through the parameters.


>> ahh did u meant ...

I mean every usage of a variable that hasn't been defined in the scope where you're using it.

hmm i'm not sure whether i pass all the parameters correctly or not...

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

void ParseInput(char *buf, char *command, char **temp, int count)
{
	int i = 0;
	int nos, s, pos;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	*command = strtok(*buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(*command != NULL)
	{
		nos = strspn (*command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++(*command);		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (*command, nl))// find if there's any new line or tab after the command
			*command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (*command) - 1;	// last character position in command
			if (*command[n] == ' ')		// if there's a space 
				*command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",*command);
		*temp [i] = *command;		// store each commands into the array
		*command = strtok(NULL,"|");
		i++;				// increment array
	}	
	count = i;
}

void ParseCommand(char *temp, char *argument, char **argv)
{
	int k = 0;
	int j = 0;
	*argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
	while (argument != NULL)
	{
		*argv[k] = *argument;			
		// store each arguments into the array
		*argument = strtok(NULL," \t\n");
		k++;
	}
}

void execute(char **argv)
{
	pid_t pid;
	int status;
	pid = fork();
	if (pid == 0)			// child
	{
		ParseCommand(temp, argument, argv);
		execvp(*argv[0],argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, command, temp, count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

>> hmm i'm not sure whether i pass all the parameters correctly or not...

If you try to compile the code - you'll get a few good indications ;)

yeah i wanted to but the system is down now... so i wasn't able to run the codes and see how~ so i wanted to know whether this is correct or is there any mistakes tat i've left out or what... but up to this point did i still do anything wrong i'm just abit confused of the pointer...even though i read the notes about pointer but when come to here i became confuse of how should i apply it...

This should clarify things :

void fun_byvalue(int value) {
    value = 10;
}

void fun_byptr(int* value) {
    *value = 15;
}

/* and then : */

int value = 5;

fun_byvalue(value);
/* value is now still 5 - the function was not able to modify it, since it was passed by value */

fun_byptr(&value);
/* value is now 15 - the function was able to modify it, since it was passed by pointer */

Select allOpen in new window

hmmm but how about for **, if i wanted to it to return the modified one to other func?
because now i'm confuse with char **temp or should i just use the normal char * temp because i've to pass it from ParseInput to execute and then to ParseCommand again?

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

void ParseInput(char *buf, char *command, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	*command = strtok(*buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(*command != NULL)
	{
		nos = strspn (*command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			*command++;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (*command, nl))// find if there's any new line or tab after the command
			*command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (*command) - 1;	// last character position in command
			if (*command[n] == ' ')		// if there's a space 
				*command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",*command);
		*temp [i] = *command;		// store each commands into the array
		*command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void ParseCommand(char *temp, char *argument, char **argv)
{
	int k = 0;
	int j = 0;
	*argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
	while (argument != NULL)
	{
		*argv[k] = *argument;			
		// store each arguments into the array
		*argument = strtok(NULL," \t\n");
		k++;
	}
}

void execute(char *temp, char **argv)
{
	pid_t pid;
	int status;
	pid = fork();
	if (pid == 0)			// child
	{
		ParseCommand(temp, argument, &argv);
		execvp(&argv[0],&argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, command, &temp, &count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(temp, &argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

Same thing :

You can pass a char* by value (so the function can't modify the pointer, but it can modify what the pointer points to) - the parameter type would be char* in that case.
Or you can pass a char* by pointer (so the function can modify the pointer, as well as what the pointer points to) - the parameter type would be char** in that case.

It all depends on what you want the function to be able to modify.

ok so that meant char **temp for
            *temp (i) = *command;            // because temp changes whenever command change,
                                                                // is it like that?
and this will works the same char **argv
              *argv[k] = *argument;      

somehow i changed the code to 2 func because b4 that was abit confuse to me... btw is there a need to put every command with * in the ParseInput func because we're pointing at the command each time to check for delimiters?

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

void ParseInput(char *buf, char *command, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	*command = strtok(*buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(*command != NULL)
	{
		nos = strspn (*command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			*command++;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (*command, nl))// find if there's any new line or tab after the command
			*command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (*command) - 1;	// last character position in command
			if (*command[n] == ' ')		// if there's a space 
				*command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",*command);
		*temp [i] = *command;		// store each commands into the array
		*command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, char *argument, char **argv)
{
	pid_t pid;
	int status;
	int k = 0;
	int j = 0;
	pid = fork();
	if (pid == 0)			// child
	{
		*argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
		while (*argument != NULL)
		{
			*argv[k] = *argument;			
			// store each arguments into the array
			*argument = strtok(NULL," \t\n");
			k++;
		}
		execvp(&argv[0],&argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
	char *command;
	char *argument;	
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, command, &temp, &count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(temp, &argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

           

oops i just realized something... hmm i actually don't need to pass the command or arguments as the parameters... can i do it something like that?

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(*buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		*temp [i] = command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, char **argv)
{
	pid_t pid;
	int status;
	int k = 0;
	int j = 0;
	char *argument;
	pid = fork();
	if (pid == 0)			// child
	{
		argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
		while (argument != NULL)
		{
			*argv[k] = argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		execvp(&argv[0],&argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, &temp, &count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(temp, &argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

>>             ParseInput(buf, &temp, &count);            // parse the user input line

temp is already a char**, which already allows you to modify the char*'s in it, so there's no need to pass it by pointer.
count has not been defined in this scope.

i got to run this code on another machine but i got a few warnings

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

void ParseInput(char *buf, char *temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = *command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, char *argv)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int j = 0;

	pid = fork();
	if (pid == 0)			// child
	{
		argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = *argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		execvp(&argv[0],&argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(temp, &argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

test.c: In function 'execute':
test.c:52: warning: cast to pointer from integer of different size
test.c: In function 'main':
test.c:87: warning: passing argument 2 of 'ParseInput' from incompatible pointer type
test.c:7: note: expected 'char *' but argument is of type 'char **'
test.c:92: warning: passing argument 1 of 'execute' from incompatible pointer type
test.c:41: note: expected 'char *' but argument is of type 'char **'
test.c:92: warning: passing argument 2 of 'execute' from incompatible pointer type
test.c:41: note: expected 'char *' but argument is of type 'char * (*)[80]'

In my previous post, I said : "temp is already a char**"
So, it IS a char**. You have to make sure to specify the right parameter type when passing it to a function.

Same with argv.

The error messages are very clear about this too :

>> note: expected 'char *' but argument is of type 'char **'

so you could have figured this out for yourself ;)

oh i thought you're saying that i've to use pass by value instead of pointer... sorry for mistaken your words....hmm somehow i got compile...but i got a segmentation fault in doing the ParseInput because it only prints the first command....

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		*temp [i] = *command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int j = 0;

	pid = fork();
	if (pid == 0)			// child
	{
		argument = strtok((char *)temp[j]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			*argv[k] = *argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		execvp(argv[0],argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command and its process id: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			execute(temp, argv); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

ls -l | more
separate by |:
ls -l
Segmentation fault

>> oh i thought you're saying that i've to use pass by value instead of pointer...

Technically, you are always passing by value.

When passing a char**, the char** is passed by value, but the char* it points to is passed by pointer. So, the char** cannot be changed by the function (because it's passed by value), but what it points to can be modified (because it's passed by pointer).

Anyway, the way you're calling the functions now, is correct.


Now, in the ParseInput function, you have this line :

>>             *temp [ i ] = *command;            // store each commands into the array

Have a close look at what's happening there, and if that's what you really intended to do ...
Take special notes of the types, and what happens when dereferencing.

oh yeah somehow i got compiled it... so now i should work on the pipe? so can i cont work like previous one just like the one that i introduced two pipes >> int fdl[2] and int fdr[2]??

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv, int count2)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	pid = fork();
	if (pid == 0)			// child
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		execvp(argv[0],argv);				
		// execute the given command
		write(1,"exec fail\n",10);			
		// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else				// parent
	{
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			execute(temp, argv, count2); 	   		// execute the command	
		}

        }
	return 0;
}

Select allOpen in new window

ls -l | more
separate by |:
ls -l
more
Command:
total 68
-rwxr-xr-x 1 cpfoo ugrad 14690 2011-04-12 15:21 a.out
-rw-r--r-- 1 cpfoo ugrad  4215 2011-04-11 07:48 as5.c
-rw-r--r-- 1 cpfoo ugrad  1837 2011-03-31 08:46 error.c
-rw-r--r-- 1 cpfoo ugrad   360 2011-03-31 08:43 LowerHalf.c
-rwxr-xr-x 1 cpfoo ugrad 14690 2011-04-12 14:54 MY
-rw-r--r-- 1 cpfoo ugrad  4128 2011-03-31 08:47 ourhdr.h
-rw-r--r-- 1 cpfoo ugrad  3403 2011-03-31 08:58 PipeScience.c
-rw-r--r-- 1 cpfoo ugrad  2542 2011-04-12 15:21 test.c
-rw-r--r-- 1 cpfoo ugrad   403 2011-03-31 08:42 UpCase.c
usage: more [-dflpcsu] [+linenum | +/pattern] name1 name2 ...
exit

somehow i started to try working on the pipe... hmm do i execute at child or parent.. because when i fork every command... the command will be the parent for the next command....so do i execute in the child first before getting it into parent?

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	int count2 = 0;
	pid_t pid;
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		int fdl[2];
		int fdr[2];
		pid = fork();
		if (pid == 0)			// first child (first command from left)
		{
			pipe (fdl);
			pid = fork();
			if (pid > 0)		// first command is new parent
			{			// first and second command share a pipe
				close (fdl[0]);			// close the read end of the pipe
				fdl[1] = dup2(fdl[1],1);	// Reasign stdout to fds[1] end of pipe
				execute(temp, argv, count2);
			}
		
			int i = count;		// number of commands
			for(j = 1; j <= i; j++)			// fork each command
			{
				count2 = j;
				if (pid == 0)			// child
				{
					pipe(fdr);
					fdr[0] = fdl[0];
					fdr[0] = dup2(fdr[0],0);// Reassign stdin to fds[0] end of pipe
					fdr[1] = fdl[1];
					close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					pipe(fdl);
					pid = fork();	// second command hatches third or following commands
					if (pid > 0)	// still in the middle commands
					{
						close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
						fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
						execute(temp, argv, count2); 	   		// execute the command	
					}
					while( j = i && pid == 0)		// in the last command 
					{
						fdr[0] = fdl[0];
						fdr[1] = fdl[1];
						close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
						fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
						execute(temp, argv, count2); 	   		// execute the command	
					}
				}
			}
		}	
        }
	return 0;
}

Select allOpen in new window

>> oh yeah somehow i got compiled it...

Are you happy with the way the code is split up now ?
Do you feel it's easier to find your way around it now ?


>> somehow i started to try working on the pipe...

I notice that the main became a LOT bigger because of this. Keep the idea of modularizing in the back of your head.

You originally had a loop that calls the execute function. That was a good idea, so try keeping it like that.

so do you meant to make a func for the pipes too? but what will be the parameters? as i thought it'll be none just like this >> void pipe() ??? or how should i do it?

I mean keep the loop like you had it before (don't use some big if-else block like you did in the last code), and integrate the use of pipes in that loop. Since you are creating pipes between every pair of consecutive processes, this should not be hard to do.

hmm i don't really get what you meant? but r u referring to his post http:#35341661 but i need loop to run for arbitrary pipes right hmmm... and other commands (not the first one) shouldn't it be inside the if else loop? or can you give some ex of how should it looks like... i can't figure out how should it be?

No, I'm referring to http:#35378183 which has the loop with the execute function being called in it - which is the good approach.
And then there's http:#35378332 where you got rid of the loop, and replaced it with a big if-else construct with several loops in there - which is a very complicated way of going about things.

I'm suggesting to start from http:#35378183, and add the use of pipes, using the same code structure - making sure not to add too much complication. Keep it simple.

>>And then there's http:#35378332 where you got rid of the loop, and replaced it with a big if-else construct with several loops in there - which is a very complicated way of going about things.

hmm i actually didn't get rid of any loop instead i just change the for loop from j = 0  to j = 1 because the first command i'll have a different pipe which do the write only, and the last command which do read only, so total i'll have 3 different procedure, shouldn't it be like that?

while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
{
	ParseInput(buf, temp, &count);		// parse the user input line
	int fdl[2];
	int fdr[2];
	pid = fork();
	if (pid == 0)			// first child (first command from left)
	{
		pipe (fdl);
		pid = fork();
		if (pid > 0)		// first command is new parent
		{			// first and second command share a pipe
			close (fdl[0]);			// close the read end of the pipe
			fdl[1] = dup2(fdl[1],1);	// Reasign stdout to fds[1] end of pipe
			execute(temp, argv, count2);
		}
		
		int i = count;		// number of commands
		for(j = 1; j <= i; j++)			// fork each command
		{
			count2 = j;
			if (pid == 0)			// child
			{
				pipe(fdr);
				fdr[0] = fdl[0];
				fdr[0] = dup2(fdr[0],0);// Reassign stdin to fds[0] end of pipe
				fdr[1] = fdl[1];
				close(fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
				pipe(fdl);
				pid = fork();	// second command hatches third or following commands
				if (pid > 0)	// still in the middle commands
				{
					close(fdl[0]);	// Not going to read in this child process, so we can close this end of pipe
					fdl[1] = dup2(fdl[1],1);	// Reassign stdout to fds[1] end of pipe
					execute(temp, argv, count2); 	   		// execute the command	
				}
				while( j = i && pid == 0)		// in the last command 
				{
					fdr[0] = fdl[0];
					fdr[1] = fdl[1];
					close (fdr[1]);		// Not going to write in this child process, so we can close this end of pipe
					fdr[0] =  dup2(fdr[0],0);	// Reassign stdin to fds[0] end of pipe
					execute(temp, argv, count2); 	   		// execute the command	
				}
			}
		}
	}	
}

Select allOpen in new window

instead of

while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			execute(temp, argv, count2); 	   		// execute the command	
		}

        }

Select allOpen in new window

or can give an example so that it is easier for me to understand?

hmm and one more thing are all the command from 2nd onwards under the first child process?

>> instead i just change the for loop from j = 0  to j = 1

If that's the only difference you see between those two blocks of code, I think you need a closer look ;)


Remember that you were trying to modularize things to avoid the complicated code that prompted you to ask the current question. So, the idea is NOT to use pretty much the exact same code as what you had originally.

Instead, call the execute function in a loop like you did, and just add a few lines of code that take care of the pipes. Don't add any if's around the loop, don't do any forks in the main, etc.

hmm u meant smt like this?

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int fds[2];
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			pipe(fds);
			close(fds[0]);
			fds[1]=dup2(fds[1],1);
			execute(temp, argv, count2); 	   		// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

but you mentioned earlier that i need to have at least 2 file descriptor for the pipes... i was abit confused here how should i put it in?
OR can i write the pipe for the first command in a separate loop?
like this

while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		pipe(fds);
		close(fds[0]);
		fds[1]=dup2(fds[1],1);
		execute(temp, argv, count2); 	   		// execute the command
		
		for(j = 1; j < i; j++)			// fork each command
		{
			int count2 = j;
			close(fds[1]);
			fds[0]=dup2(fds[0],1);
			execute(temp, argv, count2); 	   		// execute the command
		}

        }

Select allOpen in new window

>>don't do any forks in the main
but i need to fork a process for every command... hmm or do u meant that i should do the fork process in ParseCommand func?

>> hmm u meant smt like this?

Why don't you write a function that creates a new pipe, and you can then pass two extra parameters to the execute function that give the read and/or write ends that should be used for that process ?


>> but i need to fork a process for every command... hmm or do u meant that i should do the fork process in ParseCommand func?

You're already doing the fork in the execute function. There's no need to do it anywhere else.

hmm for this moment i wrote 2 func for the pipe and try to call it in the execute func but i'm not sure how will it do it for the first command? because the first command will be the child first after the fork and yet i'm doing a ReadFromPipe call from the child...

and currently now i'm using one pipe only which is int fds[2] and i'm not sure whether i pass it correctly or not in the parameters...

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv, int count2)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	pid = fork();
	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		ReadFromPipe(fds[0]);		// If is a child call the ReadFromPipe func
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		WriteToPipe(fds[1]);		// If is a parent, will write
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

void ReadFromPipe(int fds)
{
	close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
	fds[0]=dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
}

void WriteToPipe(int fds)
{
	close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
	fds[1]=dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int fds[2];
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		pipe(fds);
		
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			execute(temp, argv, count2); 	   		// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

forget to initialize the pipe so the execute func should be like this...

void execute(char **temp, char **argv, int count2)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	pid = fork();
	int fds[2];
	pipe(fds);

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		ReadFromPipe(fds[0]);		// If is a child call the ReadFromPipe func
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		WriteToPipe(fds[1]);		// If is a parent, will write
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

Select allOpen in new window

>> hmm for this moment i wrote 2 func for the pipe

I was actually referring to a function that CREATES a pipe, not two functions that USE the pipe (although you can keep them if you want).

Because you'll be creating multiple pipes, having a function that does it can be handy (especially if that function also takes care of allocating memory for the pipe).

you meant a func that just create a pipe like this?? and just call it in execute func???
hmm i'm not sure of the allocating memory for the pipe, can you teach me?

void CreatePipe(int fds)
{
      pipe (fds);
}

no, can you explain it? thanks.

>>but you need one for each pipe that you create.
the "one" you mentioned earlier is it because i only have one array? and i should do like before having 2 arrays like this... can both of the array in a same func?
void CreatePipe(int fdl, int fdr)
{
    int fdl[2];
    int fdr[2];
    pipe (fdl);
    pipe (fdr);
}

hmm no wait, i think i mistaken smt earlier, you said each pipe has to have an array of two ints and it can be done by doing the dynamically allocated memory instead of creating 4 different array for 4 different pipes is it?

so it can be done by just creating a pipe and do the dynamically allocated memory for the array in the CreatePipe func, am i right?

sorry, it should be>>>

so it can be done by just creating AN ARRAY for a pipe and do the dynamically allocated memory for the array in the CreatePipe func?

>> so it can be done by just creating AN ARRAY for a pipe and do the dynamically allocated memory for the array in the CreatePipe func?

Something like that, yes. You can dynamically create the array inside the function, and return it to the calling code.


>> no, can you explain it? thanks.

Have a look at this tutorial on how to allocate memory dynamically :

        http://www.cs.cf.ac.uk/Dave/C/node11.html

hmm i was trying to figure out how to pass the parameters for the CreatePipe but i get stucked now....
because the dynamically allocated array is an int *... and the pipe() parameter is in int????
hmmm i got no idea what should i do now?

void execute(char **temp, char **argv, int count2)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	pid = fork();
	
	CreatePipe();  // WHAT SHOULD I DO HERE? should i put fds as the parameter and initialize an 
                               // array int fds[2] before this?
                               // because i'll be using this in func for the use of pipes too

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		ReadFromPipe(fds[0]);		// If is a child call the ReadFromPipe func
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		WriteToPipe(fds[1]);		// If is a parent, will write
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

void ReadFromPipe(int fds)
{
	close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
	fds[0]=dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
}

void WriteToPipe(int fds)
{
	close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
	fds[1]=dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
}

void CreatePipe(int fds)
{
	pipe(fds);
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
}

Select allOpen in new window

>> hmm i was trying to figure out how to pass the parameters for the CreatePipe but i get stucked now....

The function to create a pipe doesn't really need a parameter.


>> because the dynamically allocated array is an int *... and the pipe() parameter is in int????

The pipe function takes an array of 2 int's as parameter.

can i do just like this?

void execute(char **temp, char **argv, int count2)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	pid = fork();
	int fds[2];
	CreatePipe();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
		fds[0]=dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		fds[1]=dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

void CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);
}

Select allOpen in new window

but i got a warning on this
test.c:83: warning: conflicting types for 'CreatePipe'     -->void CreatePipe()
test.c:51: note: previous implicit declaration of 'CreatePipe' was here    -->      CreatePipe();

since you said it's not necessary to create a func for the use of pipe i just substitute back into the execute func but did i do it correctly.... and somehow i still haven solve the problem for the first command which it doesn't read but write only, am i right?

>> can i do just like this?

You can, but you should also return the created pipe, so the calling code can use it.


>> but i got a warning on this

You probably have a declaration for the function that is inconsistent with its implementation.


>> and somehow i still haven solve the problem for the first command which it doesn't read but write only, am i right?

Treat that as an edge case. First think about how you would do it for a pipe between two processes.

So, you create the pipe with this function. And then you need to pass the write and read ends of that pipe to the right execute function somehow.

>>You can, but you should also return the created pipe, so the calling code can use it.

hmm how do i return the create pipe? can i like do return fds; at the end of the CreatePipe func?

>>You probably have a declaration for the function that is inconsistent with its implementation.

don't really what you meant here... can you be more specify?

>>Treat that as an edge case. First think about how you would do it for a pipe between two processes.

well it just needs to read input from the left and write the output it to the right...
and so do you meant that when i create this pipe at the same i'll be doing the read from pipe and write to pipe func....hmm because right now i'm doing in the child and parent process...

or just return a value indicate whether it is successful  or fail
        return 0; // if is successful

>> can i like do return fds; at the end of the CreatePipe func?

Yes. As long as you specify the correct return type for the function.


>> don't really what you meant here... can you be more specify?

Look at both lines indicated by the warning message, and see if you spot an inconsistency.


>> and so do you meant that when i create this pipe at the same i'll be doing the read from pipe and write to pipe func....hmm because right now i'm doing in the child and parent process...

You create a pipe in main, but you want to use the pipe in the execute function. So you have to pass the relevant data to the execute function.

The execute function executes one process. One process can have a read end of a pipe, and a write end of a pipe. So the execute function needs two extra parameters to pass in the read end of a pipe and the write end of a pipe. These can then be appropriately used in the function.

hmm can i do smt like this... i try to return fds with the func in int but i got a warning...

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv, int count2, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	int fds[2];
	fds[0] = r;
	fds[1] = w;
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}

		while ( n > 0)
		{
			CreatePipe();	// Create another pipe
			close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
			fds[0]=dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		}		
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		fds[1]=dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return fds;
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int r, w;
	int fds[2];
	r = fds[0];
	w = fds[1];

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			CreatePipe();
			execute(temp, argv, count2, r, w); 	   		// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

ok i changed it to this

int CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return *fds;
}

Select allOpen in new window

hmm but i think it didn't pass the fds to execute func.... no idea on this:(

>> hmm can i do smt like this... i try to return fds with the func in int but i got a warning...

What type is fds ?

is an array in int

>> is an array in int

More specifically, it's an int*.

So, what type do you have to return ?

yeah i did some changes here http:#35393360
where i return *fds;

>> yeah i did some changes here http:#35393360
>> where i return *fds;

What would be the use of only returning one int ? You'd be losing the other one.

hmm i got no idea how to return 2 ints
    return *fds[0], *fds[1]; // or should i make use of the sizeof thingi???

or just like this

int CreatePipe()
{
      int *fds = (int *)calloc(2, sizeof(int));      // Dynamically allocated memory
      pipe(fds);

      return *fds, *fds;
}

Let me repeat my earlier question/hint :

>> More specifically, it's an int*.
>> 
>> So, what type do you have to return ?

return an int *??
but i tried using return (int *)fds, (int *)fds; but this doesn't work
that's y i use return *fds, *fds; //???

>>What would be the use of only returning one int ? You'd be losing the other one.
aooww i mistaken your words again that's why i was thinking why/how to return two...

hmm ok when i first tried to return (int *)fds; //this doesn't work
so i tried it using return *fds; // this works

why is it like this, which is the correct one to do??

hmmm wait or should i use this since the beginning part is the returntype?
int *CreatePipe()
{
      int *fds = (int *)calloc(2, sizeof(int));      // Dynamically allocated memory
      pipe(fds);

      return (int *)fds;
}

>> hmmm wait or should i use this since the beginning part is the returntype?

That's it :)

hmm i not sure where's wrong because the output is not smt i want...

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

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv, int count2, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	int fds[2];
	fds[0] = r;
	fds[1] = w;
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}

		while ( n > 0)
		{
			close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
			fds[0]=dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		}		
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		fds[1]=dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int r, w;
	int fds[2];
	r = fds[0];
	w = fds[1];

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		for(j = 0; j < i; j++)			// fork each command
		{
			int count2 = j;
			CreatePipe();	// Create another pipe
			execute(temp, argv, count2, r, w);	// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

it stops at the second command...

ls -l | grep test
separate by |:
ls -l
grep test
Command:
total 68
-rwxr-xr-x 1 cpfoo ugrad 14928 2011-04-14 14:17 a.out
-rw-r--r-- 1 cpfoo ugrad  4215 2011-04-11 07:48 as5.c
-rw-r--r-- 1 cpfoo ugrad  1837 2011-03-31 08:46 error.c
-rw-r--r-- 1 cpfoo ugrad   360 2011-03-31 08:43 LowerHalf.c
-rwxr-xr-x 1 cpfoo ugrad 14690 2011-04-12 14:54 MY
-rw-r--r-- 1 cpfoo ugrad  4128 2011-03-31 08:47 ourhdr.h
-rw-r--r-- 1 cpfoo ugrad  3403 2011-03-31 08:58 PipeScience.c
-rw-r--r-- 1 cpfoo ugrad  3199 2011-04-14 14:17 test.c
-rw-r--r-- 1 cpfoo ugrad   403 2011-03-31 08:42 UpCase.c

i think the problem is in the if loop when deciding whether which one is a child or parent...
because is only when j = 1, we do the 2nd fork, first command only will be the parent(do write end of pipe) and the 2nd is the child (do read end of pipe), am i right? as this is what i thought it should be...

You call the CreatePipe function, but you don't do anything with the pipe it returns.

You have to actually use that pipe.

You'll have to also pass the right read and write end of two different pipes into the execute function (ie. the read end of the previous pipe, and the write end of the next pipe).

Note that you don't need to create any new pipes inside the execute function. You should only use these two arguments.

Do you understand how this would work ? Because it feels like you're just wandering around in the dark. I've mentioned it a few times before that you should understand what has to happen first, before actually writing code.

yes i know how it should works but i just dunno how should i pass it... like in the case of CreatePipe func, i know it returns the 2 file descriptor back to main but how should i pass it to the execute func, is there a need to have a parameter for CreatePipe func but you did mentioned earlier that there's no need of any parameters... that's why i have somekind like int r and int w that takes the fds[0] and fds[1]... i'm not sure how should i write this to pass it to execute func!

Did you read the tutorial about functions ?

Did you understand how return values work, and how you can use them ?


Suppose you have three processes, and two pipes to link them together. If you call the execute function for the middle process, which ends of which pipes does it need ?

yes i read that and i thought i can use something like this but i can only think of one, how should i pass 2 parameters to execute?
int r;
r = CreatePipe();
execute(temp, argv, count2, r, w);      

What does CreatPipe return ?
What type does it have ?

>>Suppose you have three processes, and two pipes to link them together. If you call the execute function for the middle process, which ends of which pipes does it need ?

the middle process -->when j =1--> after a fork process --> 2nd argument should read end of pipe when it's a child and write end of pipe at parent??
 

>>What does CreatPipe return ?
What type does it have ?

int *
hmm can i do like this -->
int *fds[2];
fds=CreatePipe();
execute(temp, argv, count2, fds[0], fds[1]);   // is it possible to write like this?

>> the middle process -->when j =1--> after a fork process --> 2nd argument should read end of pipe when it's a child and write end of pipe at parent??

Let's name the two pipes pipe1 and pipe2.
pipe1 is the pipe used between the first and second process, and pipe2 is the pipe used between the second an third process.

So, I'll ask the question again : If you call the execute function for the middle process, which ends of which pipes does it need ?


>> int *fds[2];

That would be an array of two pointers-to-int. Is that what CreatePipe returns ?

>>If you call the execute function for the middle process, which ends of which pipes does it need ?
it'll need both... one is with the previous command (read from) and one is with the next command (write to)

>>That would be an array of two pointers-to-int. Is that what CreatePipe returns ?
what i think is yes, it should return the array of the pipe which contains both file descriptor in it, shouldn't it?

>> it'll need both...

Both what ?

There are two pipes, and each pipe has two ends (a write and a read end). So which ends of which pipes are needed for the second process ?


>> what i think is yes, it should return the array of the pipe which contains both file descriptor in it, shouldn't it?

Be very careful with types. int* is not the same as int.
An array of int* is not the same as an array of int.

it needs a read end from the first pipe and write end to the 2nd pipe... both pipes

>>Be very careful with types. int* is not the same as int.
An array of int* is not the same as an array of int.

int * is pass by pointer so what should it will modify the data??
i'm getting confused with this...hmm can u explain or maybe example so that is easier for me to understand... cause i'm quite difficult to understand/imagine it through words...

>> it needs a read end from the first pipe and write end to the 2nd pipe... both pipes

Ok, so when calling the execute function, you need to pass in those two ends as parameters.


>> hmm can u explain or maybe example so that is easier for me to understand...

Have a detailed read through this tutorial on pointers :

        http://www.cs.cf.ac.uk/Dave/C/node10.html

and make sure that you understand it completely. Because a good understanding of pointers is absolutely necessary when programming in C.

>>Ok, so when calling the execute function, you need to pass in those two ends as parameters.
let's say for middle process, we will have two ends as parameters to pass it to execute func right but how about the first and the last command which only need one???

basically i understand but just for some simple code/examples on pointers... but when come to this pipe...i tend to get confused...

and yeah we need two pipes for every middle process, so in the for loop in main we call CreatePipe once before the execute func :
1. - when j = 0;
    - create a pipe
    - return the write end of pipe to execute func
    - fork a process and execvp
2. - when j =1;
    - create a another pipe
    - but this time have to read end of the previous pipe and write end of pipe in the newly created pipe
      ^< here's where i got confused... i know is smt to do with pointer to refer back to the previous pipe
           and i totally got no idea how to do this!
    -  fork a process and get to read end of pipe and write end of pipe , then execvp
3. - when j = 2;
    - this only needs to read from the previous pipe
    - no need to create another pipe
    - fork a process, read end of pipe, execvp

>>       ^< here's where i got confused... i know is smt to do with pointer to refer back to the previous pipe
>>            and i totally got no idea how to do this!

You can simply keep two int*'s around, called previousPipe and nextPipe. And at the end of every iteration, you copy the nextPipe variable to the previousPipe variable, and on the next iteration, you create a new nextPipe.

>>You can simply keep two int*'s around, called previousPipe and nextPipe. And at the end of every iteration, you copy the nextPipe variable to the previousPipe variable, and on the next iteration, you create a new nextPipe.

this is so confusing to me.... hmmm do i call the previousPipe and nextPipe in the execute func (is this these the 2 parameters that we're going to pass from CreatePipe?? ) and how do i copy the nextPipe variable to the previousPipe variable... this is kind like new to me....

>> hmmm do i call the previousPipe and nextPipe in the execute func (is this these the 2 parameters that we're going to pass from CreatePipe?? )

We already determined what you're supposed to pass to the execute function in http:#35396840. You said :

>>     - but this time have to read end of the previous pipe and write end of pipe in the newly created pipe

You only need two ENDS of a pipe, not two ENTIRE pipes.


>> and how do i copy the nextPipe variable to the previousPipe variable...

They're int*'s - there's nothing magical about copying an int* to another int*.

Maybe it'll be easier to understand if you keep an array of all pipes ? ie. an array of int*'s ?

yes i know we pass the ENDS of a pipe but what i confused was... in CreatePipe func, we create a pipe... and that pipe has TWO ENDS (one is read and one is write)... and then we'll pass this two int* to execute func... so we'll determine whether to use the read end or write end of this pipe in the execute func by using close or dup2, is it like this?

>>Maybe it'll be easier to understand if you keep an array of all pipes ? ie. an array of int*'s ?
this is more understandable for me:) but when we CreatePipe, the int* doesn't store in an array right, is it? so did u meant to create an array to store these int*?

>> and then we'll pass this two int* to execute func...

The int* points to the complete pipe - not just one end.

So, you don't pass an int* to the execute function, but only one end of the pipe (ie. one of the two ints pointed to by the int*).


>> so did u meant to create an array to store these int*?

Yes.

>>So, you don't pass an int* to the execute function, but only one end of the pipe (ie. one of the two ints pointed to by the int*).
ok when we create a pipe, we've this int * that points to TWO int (eg, int fds[0] and int fds[1]) but we only need to pass ONE OF THESE TWO INT to execute func... but how do we know which one to pass... do you get what i meant?

>> but how do we know which one to pass...

As determined before : You pass the read end of one pipe, and the write end of the other pipe.

because this two func are in the for loop so i easily get mess around with this...

when j = 0 (1st command), we create a pipe called pipe1 (this will return an int * that points to two int) but this time we need to pass the write end of pipe 1 to execute func...??
(how can we pass only the write end...do we specified like to pass fds[1] smt like this to execute func or how?)

(so we do need an array(int array[]) to store int *....and each int * has two element (in int))
let's say array[0] =  fds[0] - array[1] = fds[1] (both of these are the first pipe of int*) and then can we continue using it... like array[3] = fds[0] - array[4] = fds[2] (both of these are the 2nd pipe of int*) this is just my guess....

so when j = 0 we pass the array[1]
then when j = 1 we pass the array[0] and array[4]
then when j = 3 we pass the array[3]

when j = 1( 2nd command)... in the for loop we create another pipe called pipe 2 but this time we need to pass the read end of pipe 1 and write end of pipe 2 to execute func...

when j = 2 (last command)... get out of the loop since my for loop is j < i ... but we still need to pass the read end of pipe 2 to execute func...

is this how should the pipe works.... (eg. cmd1 | cmd2 | cmd3)

>> when j = 0 (1st command), we create a pipe called pipe1 (this will return an int * that points to two int) but this time we need to pass the write end of pipe 1 to execute func...??

Correct.

>> (how can we pass only the write end...do we specified like to pass fds[1] smt like this to execute func or how?)

Indeed.

>> (so we do need an array(int array[]) to store int *....and each int * has two element (in int))

You need an array of int*, so :

        int* array[SOME_SIZE];

>> let's say array[0] =  fds[0] - array[1] = fds[1]

So array[0] is the first pipe, which has a read and write end array[0][0] and array[0][1].


>> when j = 1( 2nd command)... in the for loop we create another pipe called pipe 2 but this time we need to pass the read end of pipe 1 and write end of pipe 2 to execute func...

Correct.

>> when j = 2 (last command)... get out of the loop since my for loop is j < i ... but we still need to pass the read end of pipe 2 to execute func...

Right.


>> is this how should the pipe works.... (eg. cmd1 | cmd2 | cmd3)

Yes. For three commands, that's how it would work. You can easily add more commands, or do the same for less commands.

i'm not sure but can i do i smt like this....

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int count2 = 0;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		while (j == 0)
		{

			*fds = CreatePipe();
			array[0] = *fds;
			array[0][0] = *fds[0];
			array[0][1] = *fds[1];
			read_fds = array[0][1];
			write_fds = array[0][1];
			execute(temp, argv, count2, read_fds, write_fds); // CAN I PASS THIS 2 PARAMETERS BUT ONLY USE ONE LATER IN EXECUTE FUNC WHERE I CLOSE THE READ END??
		}

		for(j = 1; j < i; j++)			// fork each command
		{
			int count2 = j;
			*fds=CreatePipe();	// Create another pipe
			array[j] = *fds;
			array[j][0] = *fds[0];
			array[j][1] = *fds[1];
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp, argv, count2, read_fds, write_fds);	// execute the command
		}
		
		while (j == i)
		{
			int count2 = j;
			read_fds = array[j-1][0];
			execute(temp, argv, count2, read_fds, write_fds);	//NOT SURE WHAT TO PASS HERE?
		}

        }
	return 0;
}

Select allOpen in new window

It's probably a good idea if you read up pointers/arrays again - I mentioned this tutorial a few posts back :

        http://www.cs.cf.ac.uk/Dave/C/node10.html

I really recommend you read through it again, and make sure you understand all of it. If there's anything you don't understand - then just ask.

i actually go through a lot of times and i understand the basic idea of how pointers work... but you know when come to a more complicated one i easily get confused of these pointers...
try to go through my comments below and see do i use it correctly...

	int *array[][2];		// can i define this array like this?
	int read_fds, write_fds;

                while (j == 0)
		{
			*fds = CreatePipe(); // CreatePipe return an array of int *fds (points to two int)
			array[0][0] = *fds[0]; // array[0][0] will store the first int * points to which is at fds[0]
			array[0][1] = *fds[1]; // is this a correct way to write like this array[0][1]??
			read_fds = *array[0][0]; // read_fds will be equal to the what array[0][0] is pointing which is the first int * points, in this case is fds[0]????
			write_fds = *array[0][1];
			execute(temp, argv, count2, read_fds, write_fds);
		}

Select allOpen in new window

hmm and when i do this       return (int *)fds;
does this return a pointer to an array of 2 ints or an array of 2 pointers to ints???

Look at a pointer as just a memory address.

You can pass that memory address around (as parameter to a function eg.), you can copy it, etc.

The memory address refers to a certain location in memory. At that location, one or more objects can be found (in this case two int's). These objects can be accessed the same way you access items in an array.

You don't need to over-complicate it. The CreatePipe function returns an int*. You can store that int* in an array of int*'s. And you can get the first and second int that the int* is pointing to, and pass it as parameter to the execute function.

so in int *array[j][2]; here we store the address of the int *...
array[j] = &fds // we store the address of the int* (consists of two int) in this array
(array[0] -> fds[0] fds[1] sp array[0] is pointing to the address of the int * that points to two int??? fds[0] and fds[1] live in int *fds)

and so i can actually remove these 2 lines because before that the array[0] is already referring  to the first and second int in the int * in this array
      array[0][0] = *fds[0];
      array[0][1] = *fds[1];

but we do need to remain these 2 lines is it to pass it as parameter to the execute func???
      read_fds = *array[j-1][0];
      write_fds = *array[j][1];

>> we store the address of the int*

an int* is ALREADY an address. There's no point in taking the address of a pointer (in this case).

>> fds[0] and fds[1] live in int *fds

Yes.

>> and so i can actually remove these 2 lines

You can remove them. You just have to store the int* returned by the CreatePipe function in the array.

>>       read_fds = *array[j-1][0];

What is the * for ? What type does array[j-1][0] have ?

so we create an array for int * with  just a normal array and not a 2dimensional array...but when we store the int * into the array will it automatically become a 2D array??

#define SIZE 10
int *array[SIZE]; //and so we'll 10 pointers to int * (each int * points to two int)???
        *fds = CreatePipe(); // return an int *
      array[j] = fds; // fds in this case is already in int * so can we straight away put like this?
                              // so the array store the int *
      read_fds = array[j-1][0];
        write_fds = array[j][1];
      execute(temp, argv, count2, read_fds, write_fds);

>> but when we store the int * into the array will it automatically become a 2D array??

You can access the objects that an int* points to, in the same way that you access items from an array.

So, if you have an array of int*'s, then you can access the objects, in the same way that you access the items of an array of arrays (a 2D array).


>> int *array[SIZE]; //and so we'll 10 pointers to int * (each int * points to two int)???

Yes.


>>         *fds = CreatePipe(); // return an int *

Why the * ?
Why not directly store it in the array ?

>>Why the * ?
Why not directly store it in the array ?

hoh yeah...
int *array[SIZE];
        *array[j] = CreatePipe(); // return an int * and store into an array
      read_fds = array[j-1][0]; // can i access the object through this way?
        write_fds = array[j][1];
      execute(temp, argv, count2, read_fds, write_fds);

>>         *array[j] = CreatePipe(); // return an int * and store into an array

Again : why the * ?

CreatePipe returns an int*.
And array is an array of int*, so array[j] is an int*.

There's no need to dereference anything.

oops and i forget to remove the * again... the below code should how it be...
ok so now here's the prob...the 1st command and the last command which only do write end and read end respectively....
for the 1st command... somehow i create a pipe and pass both ends and in the execute func i can close the read end of the pipe...
but for the last command... because there's no need to create a pipe where we just use the previous one for read, but we're only passing the previous read end of pipe , so how can i do this?

int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		while (j == 0)
		{
			array[j] = CreatePipe();
			read_fds = array[0][0];
			write_fds = array[0][1];
			execute(temp, argv, count2, &read_fds, &write_fds);
		}

		for(j = 1; j < i; j++)			// fork each command
		{
			array[j] = CreatePipe();
			read_fds = array[0][0];
			write_fds = array[0][1];
			execute(temp, argv, count2, &read_fds, &write_fds);
		}
		
		while (j == i)
		{
			int count2 = j;
			read_fds = array[j-1][0];
			execute(temp, argv, count2, &read_fds, &write_fds);	// execute the command
		}

        }

Select allOpen in new window

How about you pass an invalid value for the edge cases ?

what you meant by an invalid value, any example? and is it for the last command only or do i have to use it for both first and last command?

>> what you meant by an invalid value

Any value that is not a valid value, is an invalid value.

If you pass a value to the function that is not a valid value, the function can know that it's not supposed to use the value.

do you meant smt like
int invalid = -1;
and then i pass it to the execute func in this way
execute(temp, argv, count2, &read_fds, invalid);      // execute the command

>> int invalid = -1;

For example, yes.


>> &read_fds

Can you explain why you have the & there ?

oops oh yeah i realize the mistake because at first while i was doing the array thingi, i kind of messed around with all these.. and so in the execute func i have * for the last two parameters and i got an error... that's why when i call the execute func i put an & at the last two parameter...

ok this is the code but it's still not working yet... did i do any parts wrongly? because when i test on
ls -l | grep test it prints out ls -l numerous times????

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char **temp, char **argv, int count2, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	int k = 0;
	int n;
	n = count2;
	int fds[2];
	fds[0] = r;
	fds[1] = w;
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		
		while (n != 0)
		{
			close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
			dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	char *argv[80]={0};
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int count2 = 0;
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands
			
		while (j == 0)
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp, argv, count2, invalid, write_fds);
		}

		for(j = 1; j < i; j++)			// fork each command
		{
			count2 = j;
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp, argv, count2, read_fds, write_fds);
		}
		
		while (j == i)
		{
			count2 = j;
			read_fds = array[j-1][0];
			execute(temp, argv, count2, read_fds, invalid);	// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

Why do you have this :

>> while (j == 0)

?

How many times do you expect that loop to iterate ?

ah yeah i didn't realize that the 0 will keep going on and on...so i made some changes on the j = 0 and j = i... but it still prints out all the ls -l instead of just the one with test (ls -l | grep test)??

while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands	

		array[j] = CreatePipe();
		write_fds = array[0][1];
		execute(temp, argv, count2, invalid, write_fds);	

		for(j = 1; j < i; j++)			// fork each command
		{
			count2 = j;
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp, argv, count2, read_fds, write_fds);
		}
		
		if (j == i)
		{
			count2 = j;
			read_fds = array[j-1][0];
			execute(temp, argv, count2, read_fds, invalid);	// execute the command
		}

        }

Select allOpen in new window

>> void execute(char **temp, char **argv, int count2, int r, int w)

What are all these arguments for ?

Shouldn't you just be passing the full command (ie. including parameters) as a string, and then the two ends of the pipes ?

So, you'd basically tell the function to execute the fiven command, and to connect it to the given pipe ends. Any other parameters are unnecessary.

hmm true that the arguments is unnecessary... because i just need to pass the command and end of pipes and also the count2 that indicate which command in the array...but it still gives me the same output.... where goes wrong?

void execute(char **temp, int count2, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int n;
	n = count2;
	int fds[2];
	fds[0] = r;
	fds[1] = w;
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp[n]," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		
		while (n != 0)
		{
			close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
			dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int count2 = 0;
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands	

		array[j] = CreatePipe();
		write_fds = array[0][1];
		execute(temp, count2, invalid, write_fds);	

		for(j = 1; j < i; j++)			// fork each command
		{
			count2 = j;
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp, count2, read_fds, write_fds);
		}
		
		if (j == i)
		{
			count2 = j;
			read_fds = array[j-1][0];
			execute(temp, count2, read_fds, invalid);	// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

>> and also the count2 that indicate which command in the array..

Why not directly pass the right command ? Why should the execute function have to worry about getting the right one from the array ?

how should i do so that it only passes that specific pointer of the array to the execute func??
can i do smt like
      execute(*temp[j], count2, read_fds, write_fds);

but in the func there i have char **temp for this??
or should i create smt and then store this particular temp[j] into it then only pass to execute func?

i'm not sure whether i can do it this way... hmmm but somehow the result is still the same...

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int n = 0;
	int fds[2];
	fds[0] = r;
	fds[1] = w;
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		
		while (n != 0)
		{
			close(fds[1]);	// Not going to write in this child process, so we can close this end of pipe
			dup2(fds[0],0); 	// Reassign stdin to fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(fds[0]); 	// Not going to write in this child process, so we can close this end of pipe
		dup2(fds[1],1); 	// Reassign stdout to fds[1] end of pipe
		while (wait(&status) != pid); 	// wait for completion
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands	

		array[j] = CreatePipe();
		write_fds = array[0][1];
		execute(temp[0], invalid, write_fds);	

		for(j = 1; j < i; j++)			// fork each command
		{
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp[j], read_fds, write_fds);
		}
		
		if (j == i)
		{
			read_fds = array[j-1][0];
			execute(temp[j], read_fds, invalid);	// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

>> i'm not sure whether i can do it this way...

That simplifies things a bit, doesn't it ?

Now, in the execute function you have :

>>       int fds[2];
>>       fds[0] = r;
>>       fds[1] = w;

Why is that ? Why not use r and w directly ?

You also have :

>>             while (n != 0)

What is that for ?

And finally :

>>             while (wait(&status) != pid);       // wait for completion

Why is that ?


Note that you'll also have to check whether the read and/or write ends passed into the execute function are valid or invalid. Note that if they're invalid, you can't use them !!


And then there are a few things to fix in main :

1) make sure that you don't call execute too many times ... There are 'count' processes to create, not one more.
2) you need to also account for the possibility of having only 1 or two processes. You can't assume there will always be at least 3.

oops i should have remove that since i'm passing an invalid value so i can actually ignore whether is it the first or last command...

>>Note that you'll also have to check whether the read and/or write ends passed into the execute function are valid or invalid. Note that if they're invalid, you can't use them !!

how should i check whether is it valid or invalid, if is invalid how should i ignore it?

>>make sure that you don't call execute too many times ... There are 'count' processes to create, not one more.
the count = i; and count is started from 0 (same as i) so it will not create one more process ryte?? if i have 2 commands then it'll only call twice the execute func and have 2 processess...

                array[j] = CreatePipe();
            write_fds = array[0][1];
            execute(temp[0], invalid, write_fds);      

            for(j = 1; j < i; j++)                  // fork each command
            {
                  array[j] = CreatePipe();
                  read_fds = array[j-1][0];
                  write_fds = array[j][1];
                  execute(temp[j], read_fds, write_fds);
            }
            
            if (j == i)
            {
                  read_fds = array[j-1][0];
                  execute(temp[j], read_fds, invalid);      // execute the command
            }

>>you need to also account for the possibility of having only 1 or two processes. You can't assume there will always be at least 3.

hmm because if i = 1 ( as i stated earlier i started from 0 - 2 commands) so the first command will first do executed func and because it fails to enter the for loop (since j = 1 is not less than i = 1) so it will enter the if loop which is the last command? shouldn't it be like this???

ah ha i know why mentioned that earlier... because i have an i++ before end of the loop so that's y i'll have one more extra....so i added this
      i--;
      *count = i;

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	i--;
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	int status;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int n = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		
		close(w);	// Not going to write in this child process, so we can close this end of pipe
		dup2(r,0); 	// Reassign stdin to fds[0] end of pipe
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		close(r); 	// Not going to write in this child process, so we can close this end of pipe
		dup2(w,1); 	// Reassign stdout to fds[1] end of pipe
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands	

		array[j] = CreatePipe();
		write_fds = array[0][1];
		execute(temp[0], invalid, write_fds);	

		for(j = 1; j < i; j++)			// fork each command
		{
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp[j], read_fds, write_fds);
		}
		
		if (j == i)
		{
			read_fds = array[j-1][0];
			execute(temp[j], read_fds, invalid);	// execute the command
		}

        }
	return 0;
}

Select allOpen in new window

>> how should i check whether is it valid or invalid,

I'm sure you can figure that out for yourself, since you chose the invalid value yourself.

>> if is invalid how should i ignore it?

Don't use it.


>> if i have 2 commands then it'll only call twice the execute func and have 2 processess...

Try counting it for yourself. If you have two processes, what will 'count' be set to ? And how many times will the execute function be called ? Look very closely at the code.


>> hmm because if i = 1 ...<SNIP> ... so it will enter the if loop which is the last command? shouldn't it be like this???

So how many times will the execute function be called if count (or i) is 1 ?

hmm can i do smt like this

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	i--;
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		int i = count;		// number of commands	

		array[j] = CreatePipe();
		write_fds = array[0][1];
		execute(temp[0], invalid, write_fds);	
                j++;

		for(j = 1; j < i; j++)			// fork each command
		{
			array[j] = CreatePipe();
			read_fds = array[j-1][0];
			write_fds = array[j][1];
			execute(temp[j], read_fds, write_fds);
		}

		read_fds = array[j-1][0];
		execute(temp[j], read_fds, invalid);	// execute the command

        }
	return 0;
}

Select allOpen in new window

>>Try counting it for yourself. If you have two processes, what will 'count' be set to ? And how many times will the execute function be called ? Look very closely at the code.

the two processes you meant is it the process after i fork in execute func??

okay so let's say i have 2 command... in ParseInput func...we will use temp[0] and temp[1] but because i have an i++ there, so i added an i-- after the loop and make count = i (both count and i starts from 0)...
back to main...
i have int i = count (so if i have 2 commands then i = 1 where count = 1)
when j = 0 (first command)...go here (because initialize j = 0)
                array[j] = CreatePipe();
            write_fds = array[0][1];
            execute(temp[0], invalid, write_fds);      // go to execute func
finish execute func come back to main...
i have a j++ (so now j = 1)
but because i = 1 so it fails to enter the for loop (as j < i) and hence it enters here where

            read_fds = array[j-1][0];
            execute(temp[j], read_fds, invalid);      // execute the command

>> so i added an i-- after the loop and make count = i (both count and i starts from 0)...

Are you sure that's what you want ? Shouldn't count be 2 ? Since you have two commands ? Otherwise why would you call it 'count' if it's not the number of commands ?

Note that you should also consider the case where there's only one command.

but i did initialize count = 0, so it should be starting 0, 1, 2 right? hmm ok i change to like this so it'll be easier to understand...
if i have 2 commands... count = 2 and will store in temp[0] and temp[1]
which i'll have j < count -1 in the for loop and j == count -1 in the if statement

>>Note that you should also consider the case where there's only one command.
i was thinking about this problem too so i added an if else statement

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1;

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		
		if (count == 1)
			execute(temp[j], invalid, invalid);
			break;
		else 
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp[j], invalid, write_fds);	
			j++;
 
			for(j = 1; j < i - 1; j++)			// fork each command
			{
				array[j] = CreatePipe();
				read_fds = array[j-1][0];
				write_fds = array[j][1];
				execute(temp[j], read_fds, write_fds);
			}
		
			if (j == i - 1)
			{
				read_fds = array[j-1][0];
				execute(temp[j], read_fds, invalid);	// execute the command
			}
		}
        }
	return 0;
}

Select allOpen in new window

hmm i forgot to change smt.. this should how it be... but i still can't figure out what's wrong:(

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	printf("separate by |: \n");
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			

		printf("%s\n",command);
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 

	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{
		ParseInput(buf, temp, &count);		// parse the user input line
		printf("Command: \n");
		
		if (count = 1)
		{
			execute(temp[j], invalid, invalid);
		}
		else 
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp[j], invalid, write_fds);	
			j++;
 
			for(j = 1; j < count - 1; j++)			// fork each command
			{
				array[j] = CreatePipe();
				read_fds = array[j-1][0];
				write_fds = array[j][1];
				execute(temp[j], read_fds, write_fds);
			}
		
			if (j == count - 1)
			{
				read_fds = array[j-1][0];
				execute(temp[j], read_fds, invalid);	// execute the command
			}
		}
        }
	return 0;
}

Select allOpen in new window

>> if i have 2 commands... count = 2 and will store in temp[0] and temp[1]

That's the standard way of doing things :)

>> which i'll have j < count -1 in the for loop and j == count -1 in the if statement

sounds appropriate.

>> i was thinking about this problem too so i added an if else statement

And how about if there are 0 commands ? When programming, you need to really consider every single scenario !


>> but i still can't figure out what's wrong:(

You need to keep track of what the child process is, and what the parent process is ... And make sure that you attach the pipe ends to the right process !!

i added an else statement...i'm not sure whether it is an appropriate way to do it like or not...

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 

	printf("prompt->");
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{

		ParseInput(buf, temp, &count);		// parse the user input line
	
		if (count = 1)
		{
			execute(temp[j], invalid, invalid);
		}
		if (count > 1)
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp[j], invalid, write_fds);	
			j++;
 
			for(j = 1; j < count - 1; j++)			// fork each command
			{
				array[j] = CreatePipe();
				read_fds = array[j-1][0];
				write_fds = array[j][1];
				execute(temp[j], read_fds, write_fds);
			}
		
			if (j == count - 1)
			{
				read_fds = array[j-1][0];
				execute(temp[j], read_fds, invalid);	// execute the command
			}
		}
		else
		{
			printf("prompt->");
		}
        }
	return 0;
}

Select allOpen in new window

the child process will do the read end(so i close the write end and dup2 the read end) and in the parent process it will do the write end(so i close the read end and dup2 the write end)....hmmm but how do i keep track it??don't really get what you meant here...

>> if (count = 1)

That's an assignment, not a comparison !


>> the child process will do the read end(so i close the write end and dup2 the read end) and in the parent process it will do the write end(so i close the read end and dup2 the write end)....

Why is that ?
Shouldn't both ends be connected to the same process ?

hmm and so i tried to put loop before the while (line:105)
      while (strlen (buf) == 0)
      {
            printf("prompt->");
      }
but it doesn't work....hmmm what should i do...because it is not printing out the printf?

>>Why is that ?
>>Shouldn't both ends be connected to the same process ?
because at first i thought that the child process only do the read end...so the child process should do both read end and write end...and so the parent process will it do anything?

hmm i changed it to this, because i realized that it doesn't make any changes...but is still not working
      if (buf[0] == '\0')
      {
            printf("prompt-> %s", buf);
      }

i changed it again... but it prints out this prompt-> ???      ? (before any input)
printf("prompt-> %s", buf);
      while (buf[0] = '\0')
      {
            printf("prompt-> %s", buf);
      }
      

oops sorry i made another mistake again it is a printf and not scanf...so it should be like this...but is not working too:(
printf("prompt->");
      scanf("%s", buf);
      while (buf[0] = '\0')
      {
            printf("prompt-> %s", buf);
      }

ok finally figure out the problem...but i still can't get this
>>Why is that ?
>>Shouldn't both ends be connected to the same process ?
because at first i thought that the child process only do the read end...so the child process should do both read end and write end...and so the parent process will it do anything?

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

#define SIZE 100

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
	else	/* This is the parent process */
	{

	}
	
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 
	char *p;
	
	printf("prompt->");
	while(strcmp(fgets((char *)buf, 80, stdin), "\n") ==0)  // user command line
	{
		printf("prompt->");
	}
	
	while(strcmp(fgets((char *)buf, 80, stdin), "exit\n")!=0)  // user command line
	{

		ParseInput(buf, temp, &count);		// parse the user input line
	
		if (count = 1)
		{
			execute(temp[j], invalid, invalid);
		}
		if (count > 1)
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp[j], invalid, write_fds);	
			j++;
 
			for(j = 1; j < count - 1; j++)			// fork each command
			{
				array[j] = CreatePipe();
				read_fds = array[j-1][0];
				write_fds = array[j][1];
				execute(temp[j], read_fds, write_fds);
			}
		
			if (j == count - 1)
			{
				read_fds = array[j-1][0];
				execute(temp[j], read_fds, invalid);	// execute the command
			}
		}
        }
	return 0;
}

Select allOpen in new window

>> because at first i thought that the child process only do the read end...so the child process should do both read end and write end...and so the parent process will it do anything?

As I said : you need to make sure you know what each process is.

What is the child process ? What's its purpose ?
What is the parent process ? What's its purpose ?

in the child process, we have to read end of the previous pipe as stdin and write end to the next pipe as stdout.. and the parent process have to write to the next child's process input and read from the previous child's process output???

and about the command = 0, because right now i've to input twice the command, is there any way to make it so that i only need to input once? because the prob here is i have 2 fgets and i've tried various ways but still can't...

>> in the child process, we have to read end of the previous pipe as stdin and write end to the next pipe as stdout.. and the parent process have to write to the next child's process input and read from the previous child's process output???

Which is the process that executes the command ?
Which is the process that the pipes whould be attached to ?
Does the parent process ever execute a command ?

>>Which is the process that executes the command ?
the child process should execute the command...

>>Which is the process that the pipes whould be attached to ?
in this case both of the process (child and parent) will be attaching to the two pipes(pipe 1 and pipe 2)

>>Does the parent process ever execute a command ?
hmm i don't think so...

>> in this case both of the process (child and parent) will be attaching to the two pipes(pipe 1 and pipe 2)

Why would a pipe need to be attached to the parent process if you say that :

>> >>Does the parent process ever execute a command ?
>> hmm i don't think so...

so the parent process will do nothing... we only do the pipe attaching in child process and execute the command?

>> so the parent process will do nothing...

All the parent does, is parse the user input, and create the pipes and child processes, right ?
The pipes should be between the child processes - the parent is only managing everything ...

But we parse the user input in main before calling execute func right... or do you meant to parse the command into arguments?  I thought we create a pipe before calling the execute function that's why we"re passing the 2 return values from create pipe to execute func...

Hmm what should I do so thaT that the parent process read the child process output and write the child process input??

>> But we parse the user input in main before calling execute func right..

Do you think the main is in a different process than what you refer to as the parent process ?


>> Hmm what should I do so thaT that the parent process read the child process output and write the child process input??

Why would you want that ?

>>Do you think the main is in a different process than what you refer to as the parent process ?
do you meant that they're the same process...hmmm i don't really understand here...ahh hmmm but let me think.... so in main is the parent process....so we do parseinput, create pipes and go to execute func which we have fork in it...and there's where we created our child process...and we'll do the read end and write end of pipe in this child process is it like this? i tend to mixed up everything because before that everything was in main....
so we actually don't need this line in the execute func because is just all the child process only?
else      /* This is the parent process */
{
}
      

and so in the execute func and main i've this

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}
int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 
	char *p;
	
	printf("prompt->");
	while (strcmp(fgets((char *)buf, 80, stdin), "\n") ==0)  // user command line
	{
		printf("prompt->");
	}

	if(strcmp(buf, "exit\n")!=0)  // user command line
	{

		ParseInput(buf, temp, &count);		// parse the user input line
	
		if (count = 1)
		{
			execute(temp[j], invalid, invalid);
		}
		if (count > 1)
		{
			array[j] = CreatePipe();
			write_fds = array[0][1];
			execute(temp[j], invalid, write_fds);	
			j++;
 
			for(j = 1; j < count - 1; j++)			// fork each command
			{
				array[j] = CreatePipe();
				read_fds = array[j-1][0];
				write_fds = array[j][1];
				execute(temp[j], read_fds, write_fds);
			}
		
			if (j == count - 1)
			{
				read_fds = array[j-1][0];
				execute(temp[j], read_fds, invalid);	// execute the command
			}
		}
        }
	return 0;
}

Select allOpen in new window

>> so in main is the parent process....so we do parseinput, create pipes and go to execute func which we have fork in it...and there's where we created our child process...and we'll do the read end and write end of pipe in this child process is it like this?

Yes. Calling a function does not create a new process. The function is executed within the same process.
The fork call is what creates a new process. It keeps the current process, and creates a new child process.

so is it the way i attached the two end of pipes in a wrong way or what... because it is still nor working??
if (pid == 0)      /* This is the child process */
      {
            argument = strtok((char *)temp," \t\n");      // tokenize the command into arguments
            while ((char *)argument != NULL)
            {
                  argv[k] = (char *)argument;                  
                  // store each arguments into the array
                  argument = strtok(NULL," \t\n");
                  k++;
            }
            if (r != -1)
            {
                  close(w);      // Not going to write in this child process, so we can close this end of pipe
                  dup2(r,0);       // Reassign stdin to r = fds[0] end of pipe
            }
            if (w != -1)
            {
                  close(r);       // Not going to write in this child process, so we can close this end of pipe
                    dup2(w,1);       // Reassign stdout to w = fds[1] end of pipe
            }
            execvp(argv[0],argv);            // execute the given command
            write(1,"exec fail\n",10);      // print this message if fails to execvp
            exit(1);
      }

>> because it is still nor working??

A description of how it's not working would be nice ;) The output you get would be nice too.

prompt->ls -l | grep test
cpfoo@css01:~/CS224/as5$ total 68 <-- not sure why this appeared here??
-rwxr-xr-x 1 cpfoo ugrad 14828 2011-04-19 07:59 a.out
-rw-r--r-- 1 cpfoo ugrad  4215 2011-04-11 07:48 as5.c
-rw-r--r-- 1 cpfoo ugrad  1837 2011-03-31 08:46 error.c
-rw-r--r-- 1 cpfoo ugrad   360 2011-03-31 08:43 LowerHalf.c
-rwxr-xr-x 1 cpfoo ugrad 14690 2011-04-12 14:54 MY
-rw-r--r-- 1 cpfoo ugrad  4128 2011-03-31 08:47 ourhdr.h
-rw-r--r-- 1 cpfoo ugrad  3403 2011-03-31 08:58 PipeScience.c
-rw-r--r-- 1 cpfoo ugrad  3419 2011-04-19 07:59 test.c
-rw-r--r-- 1 cpfoo ugrad   403 2011-03-31 08:42 UpCase.c
and the output stops here???

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

#define SIZE 80

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove any new lines, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");
		i++;				// increment array
	}	
	*count = i;
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);

	return (int *)fds;
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 
	char *p;
	
	printf("prompt->");
	while (strcmp(fgets((char *)buf, 80, stdin), "\n") ==0)  // user command line
	{
		printf("prompt->");
	}

	if(strcmp(buf, "exit\n")!=0)  // user command line
	{

		ParseInput(buf, temp, &count);		// parse the user input line
	
		if (count = 1)				// for one command
		{
			execute(temp[j], invalid, invalid);	// call execute func
		}
		if (count > 1)				// for more than one command (with pipes)
		{
			array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
			write_fds = array[0][1];	// copy the write end of the pipe to write_fds
			execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
			j++;					// increment of j
 
			for(j = 1; j < count - 1; j++)			// for each command
			{
				array[j] = CreatePipe();		// create another pipe
				read_fds = array[j-1][0];		// copy the read end of previous pipe
				write_fds = array[j][1];		// copy the write end of current pipe
				execute(temp[j], read_fds, write_fds);	// call the execute func
			}
		
			if (j == count - 1)				// for the last command
			{
				read_fds = array[j-1][0];		// copy the read end of previous pipe
				execute(temp[j], read_fds, invalid);	// call the execute func
			}
		}
        }
	return 0;
}

Select allOpen in new window

and i think the problem happen to be in btw line 102 - 107 for this
cpfoo@css01:~/CS224/as5$ total 68 <-- not sure why this appeared here??
and i think the 2nd command wasn't working with the first command... because it should only prints
-rw-r--r-- 1 cpfoo ugrad  3419 2011-04-19 07:59 test.c
for ls -l | grep test

Please have a look at the first remark I made in http:#35414863.


And the "cpfoo@css01:~/CS224/as5$" part is there because you ended the parent process before the child processes had ended.

>>And the "cpfoo@css01:~/CS224/as5$" part is there because you ended the parent process before the child processes had ended.

so i can actually put a waitpid before the return 0; so it'll wait for the child to terminate first before it terminates??

>>>> if (count = 1)
>>That's an assignment, not a comparison !
i have no idea on this other than assigning an if statement there for it to compute...any suggestions that what i can do?

>> so i can actually put a waitpid before the return 0; so it'll wait for the child to terminate first before it terminates??

If you want to, you can wait for all child processes to terminate, yes.


>> i have no idea on this other than assigning an if statement there for it to compute...any suggestions that what i can do?

Do you know the difference between = and == ?

as what i thought was = is more to assigning a value to it like int a = 0 and == is more to like a comparison of the LHS and RHS, return turn if is the same and false if it is not...
but sometimes i'll get confused on when to use = and when to use == ...

don't really understand why the = and == sign...and when i changed it to count == 1 i got this output...can you explain why that makes a difference??

prompt->ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  3986 2011-04-19 09:34 test.c

and how can i do so that after executing a given command...i can have another prompt again for user to input another command and execute it?

>> as what i thought was = is more to assigning a value to it like int a = 0 and == is more to like a comparison of the LHS and RHS, return turn if is the same and false if it is not...

That's exactly the difference.
= is for assignment, and == is for comparison.


>> but sometimes i'll get confused on when to use = and when to use == ...

When you want to assign, you use =, and when you want to compare you use ==. There's nothing magical about it ;) You just have to use the right operator.


>> and when i changed it to count == 1 i got this output...

That's the output you expected, isn't it ?


>> and how can i do so that after executing a given command...i can have another prompt again for user to input another command and execute it?

Add one more loop.

>>That's the output you expected, isn't it ?
yup!

>>Add one more loop.
hmm where should i add this loop...because currently i have a loop that takes an empty string...and so it'll keep printing prompt-> if the user didn't enter anything... if the user input a command we'll straight to the next if loop which compare the user input whether it is exit or not...if is exit it'll terminates if not we'll do the rest...and now what i need to do is to be able to print the prompt and take the user input and run it all over again...

>> and now what i need to do is to be able to print the prompt and take the user input and run it all over again...

Then do that :)

Have a loop around the code that displays the prompt, gets the user input, and processes that input. End the loop whenever the input equals "exit".

i was thinking of doing smt like this so can do fgets once and compare it with 2 things either is 0 command or exit...but i seems to have some prob with it...how can i solve this?

      printf("prompt->");
      while (strcmp(fgets((char *)buf, 80, stdin), "\n" || "exit\n") ==0)  // user command line
      {
            if(strcmp(buf, "\n")==0)
            {
                  printf("prompt->");
            }
      

              if(strcmp(buf, "exit\n")!=0)  // user command line
             {
               }
        }

i tried to do it like this...but after they found a command and execute it...it didn't back to the while loop... hmm what should i do in order to make it loop back again?? shouldn't the while loop will keep going on until they find the exit?

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int *fds[2];
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 
	int p;
	int status;

	printf("prompt->");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{

		if(strcmp(buf, "\n")!=0)  // user command line
		{
			ParseInput(buf, temp, &count);		// parse the user input line
	
			if (count == 1)				// for one command
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}
			while (wait(&status) != pid); 	// wait for completion
                        printf("prompt->");
        	}
		else 
			printf("prompt->");

	}

	return 0;
}

Select allOpen in new window

prompt->
prompt->ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  4045 2011-04-19 11:10 test.c

>> while (wait(&status) != pid);       // wait for completion

What are you waiting for here ?

status has never been initialized ... nor has it ever been used in any way.

hmm since i changed the = to == it didn't have that prob...so i just removed the wait line...
but i still can't get to loop back again ... but if i enter exit after the command being execute... it does terminate the prog... but if i enter another command....it does nothing....

>> but if i enter exit after the command being execute... it does terminate the prog...

Does that surprise you ? (I assume you do know what the exit function does)


>> but if i enter another command....it does nothing....

Look at all the variables that you initialized outside of the loop. Maybe some of those should be (re-)initialized inside the loop.

Think logically and carefully about what the code is doing.

>>Does that surprise you ? (I assume you do know what the exit function does)
i know what the exit func does.... but i don't really understand was why it only does for the exit but not a command? let's say my output is smt like this

prompt-> (press enter)
prompt-> (press enter)
prompt->ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  3964 2011-04-19 14:29 test.c
(enter) <-- don't really understand why the prompt is not printed in this line but prints only after enter
prompt-> (enter)
prompt->ls -l
(enter) <-- doesn't do anything
prompt-> (enter)
prompt->exit <-- this terminates the program

Please read the rest of my previous post ;)

>>Look at all the variables that you initialized outside of the loop. Maybe some of those should be (re-)initialized inside the loop.

hmm do you meant that i should re-initialized the char buf[80] because i'll have to reuse this buf to store another new command?

hmmm and what's the purpose of re-initialized the variables?

>> hmm do you meant that i should re-initialized the char buf[80]

Is that the only variable you have defined outside of the loop ?


>> hmmm and what's the purpose of re-initialized the variables?

What's the purpose of initializing each of those variables ?
The purpose of re-initializing it, is the same.

here i have the code...but is there any ways to solve the extra line that appear right after the command being executed

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int *
	int invalid = -1; 

	printf("prompt-> ");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{
		if(strcmp(buf, "\n")!=0)  // user command line
		{
			memset(temp,0,sizeof(temp));
			memset(array,0,sizeof(array));
			int count = 0;
			int j = 0;
			ParseInput(buf, temp, &count);		// parse the user input line
	
			if (count == 1)				// for one command
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}
        	}
		else 
			printf("prompt-> ");
		memset(buf,0,sizeof(buf));
	}
	return 0;
}

Select allOpen in new window

prompt->
prompt-> ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  4114 2011-04-19 15:44 test.c
                  [when i press enter only the prompt below will appear?]
prompt->
prompt-> ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  4114 2011-04-19 15:44 test.c

prompt-> ls -l
total 88
-rwxr-xr-x 1 cpfoo ugrad 14888 2011-04-19 15:44 a.out
-rw-r--r-- 1 cpfoo ugrad  4215 2011-04-11 07:48 as5.c
-rw-r--r-- 1 cpfoo ugrad  1837 2011-03-31 08:46 error.c
-rw-r--r-- 1 cpfoo ugrad   360 2011-03-31 08:43 LowerHalf.c
-rwxr-xr-x 1 cpfoo ugrad 14690 2011-04-12 14:54 MY
-rwxr-xr-x 1 cpfoo ugrad 14828 2011-04-19 08:10 myshell
-rw-r--r-- 1 cpfoo ugrad  4128 2011-03-31 08:47 ourhdr.h
-rw-r--r-- 1 cpfoo ugrad  3403 2011-03-31 08:58 PipeScience.c
-rw-r--r-- 1 cpfoo ugrad  4114 2011-04-19 15:44 test.c
-rw-r--r-- 1 cpfoo ugrad   403 2011-03-31 08:42 UpCase.c

prompt-> exit

and because my shell need to include for redirection too...> and < so how can i do for this?
i know it almost work the same as pipe as we need dup2... but it'll be quite difficult ryte...because at first i parse the user input is based on | and now i've to add more things in now so that it can differentiate this three ryte ( the | , <, >)...
hmm or should i close this post and open another post to ask for this question?

>> but is there any ways to solve the extra line that appear right after the command being executed

You're only showing the prompt if the previous command was empty.
You should probably also show it if the previous command was not empty.


>> but it'll be quite difficult ryte..

No. Now that you have a nice setup that splits up the functionality in separate functions, it'll be easy to add.

The execute function for example already takes care of redirecting stdin and/or stdout to the specified file descriptors.
So, if redirection to/from file is used for a command, you just have to make sure to create/open that file, and pass its file descriptor to the execute function.


>> hmm or should i close this post and open another post to ask for this question?

You can keep this question open, if you promise to do the large part of the work yourself now. I will do a lot less hand-holding this time, so you'll have to do a bit more research on your own.

That's a large part of writing code : encountering problems, and figuring out how to solve them.

I'm confident that you can do this on your own ! I'll be here to support you if you really need it, but please only ask me something after you have tried looking for the solution yourself (Google is your friend, as are the man pages for the used functions).

Just one hint : you currently have the CreatePipe function that returns two file descriptors. It would be nice to add two more functions, called something like CreateInputRedirectFile and CreateOutputRedirectFile that each return one file descriptor for the cases < and > resp.

I've done with the 2 functions but i'm not sure is it correct to it in this way...

int *CreateInputRedirectFile(char *temp)
{
	int fdi;					// file descriptor
	if (fdi = open(temp, O_RDONLY) < 0)	// Open the input files
		printf("Open input fail");		// Error
	if (dup2(fdi, 0) < 0)				// Replace stdin with input file
		printf("Redirect input fail");		// Error

	return (int *)fdi;				// Return a file descriptor
}

int *CreateOutputRedirectFile(char *temp)
{
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;
	int fdo;					// file descriptor
	if (fdo = open(temp, O_CREAT | O_RDWR | O_TRUNC, mode) < 0)	// Open the output files
		printf("Open file fail");		// Error
	if (dup2(fdo,1) < 0)				// Replaces stdout with output files
		printf("Redirect output fail");		// Error

	return (int *)fdo;				// Return a file descriptor
}

Select allOpen in new window

here i got some problem.... because previously i have the ParseInput func that tokenized the command line when it sees a |... so now do i need to have another one to tokenized for < or >...and in the main how should i keep track it... because < works another way ... hmm i was kind of confused with this... and i couldn't find any example that explains this very well... can you explain to me? thanks:)

There aren't supposed to be any dup calls in those functions - that's supposed to happen in the child process (and you've already got code for that in the execute function).

These two functions are supposed to be called BEFORE the execute function - just like the CreatePipe function.


>> so now do i need to have another one to tokenized for < or >...and in the main how should i keep track it... because < works another way ... hmm i was kind of confused with this... and i couldn't find any example that explains this very well... can you explain to me? thanks:)

Tokenizing the input with | as delimiter is still ok, because it splits up the input in separate commands.
Now, what you need to do additionally, is check each command for the presence of < or >, and open the appropriate files.

correct me if i'm wrong...
at first i was thinking to use the same method as i did for the ParseInput for | (this store in temp array) and do it for < and > and then store it in temp2 array and temp3 array respectively
like for example ls -l > file2 | grep test
using ParseInput func I'll have temp[0] = ls -l > file2 and temp[1] = grep test and so i'll check further for any > or < and so i'll have temp2[0] = ls -l , temp2[1] = file2 and temp2[3] = grep test
BUT the problem is how am i going to keep track which one(|, < or >) come first... and call the right func???
(just like in this case i'll have to call the CreateOutputRedirectFile first)

and because currently i have this... the return value from CreatePipe is store in this array...what should i do so that array can store either 3 of these return value (CreatePipe, CreateInputRedirectFile, CreateOutputRedirectFile) accordingly??

array[j] = CreatePipe();            // create another pipe
read_fds = array[j-1][0];            // copy the read end of previous pipe
write_fds = array[j][1];            // copy the write end of current pipe

i'm sorry for asking so many questions but i seriously got confused with this...

>> ls -l > file2 | grep test

That wouldn't make sense, because you're trying to redirect the stdout of the first command (ls -l) to both the file2, and to the second command.

This would make more sense as an example :

        ls -l | grep test > file2


>> BUT the problem is how am i going to keep track which one(|, < or >) come first... and call the right func???

Ok. Maybe the following approach will be easier to understand.

How about keeping the ParseInput function the way it is, so in main you get the separate command (with redirections to/from file still in there). So in main you'd get "ls -l" and "grep test > file2".

Then you pass these commands on to the execute function (just like you did before).

So far nothing has changed.

Now, in the execute function, you're already splitting up the command in arguments. It should be easy to add a check to see whether the argument starts with a > or a <. If that's the case, you look for the filename that follows it, and call the appropriate CreateInputRedirectFile or CreateOutputRedirectFile function.

Note that :
(a) this resolves the priority problem, since you can simply overwrite whatever is in r and/or w.
(b) you shouldn't pass the >, < and filenames as parameters to the exec function.

ah ha now i understand:)
hmm do you think is it appropriate to do it in this way? i'm not sure whether i should pass the int r or int w as the parameter...but because if previously either one of them is invalid..and so i want it to change by calling either the CreateInputRedirectFile or CreateOutputRedirectFile and get a valid r or w??

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char input = ">";
	char output = "<";
	int k = 0;
	int m = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		for(k = 0; k < m; k++)
		{
			if (strcmp(argv[k],input)==0)
			{
				CreateInputRedirectFile(argv[k+1], r); //HERE i think i should pass the next 
                                                                                             // argument and get a return value for r since 
                                                                                             // redirect input only does read?
			}
			else if (strcmp(argv[k], output)==0)
			{
				CreateOutputRedirectFile(argv[k+1], w); /HERE i think i should pass the next 
                                                                                           // argument and get a return value for w since 
                                                                                             // redirect output only does write?
			}
		}
		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int CreateInputRedirectFile(char *argv, int fdi)
{
	if (fdi = open(argv, O_RDONLY) < 0)	// Open the input files
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argv, int fdo)
{
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;
	int fdo;					// file descriptor
	if (fdo = open(argv, O_CREAT | O_RDWR | O_TRUNC, mode) < 0)	// Open the output files
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

CreateInputRedirectFile and CreateOutputRedirectFile should RETURN the file descriptor, because it's a RESULT of the function call.

hmm return the file descriptor and putting it as parameter for this case is it the same...because i want it to return a file descriptor that will write over the previous w or r???

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = ">";
	char *output = "<";
	int k = 0;
	int m = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}
		for(k = 0; k < m; k++)
		{
			if (strcmp(argv[k],input)==0)
			{
				CreateInputRedirectFile(argv[k+1], r);
			}
			else if (strcmp(argv[k], output)==0)
			{
				CreateOutputRedirectFile(argv[k+1], w);
			}
		}
		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}
		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);					// Create a pipe

	return (int *)fds;				// Return the two file descriptors
}

int CreateInputRedirectFile(char *argv, int fdi)
{
	if (fdi = open(argv, O_RDONLY) < 0)	// Open the input files
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argv, int fdo)
{
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;
	if (fdo = open(argv, O_CREAT | O_RDWR | O_TRUNC, mode) < 0)	// Open the output files
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

or no should i do it in this way

for(k = 0; k < m; k++)
		{
			if (strcmp(argv[k],input)==0)
			{
				r = CreateInputRedirectFile(argv[k+1]);
				
			}
			else if (strcmp(argv[k], output)==0)
			{
				w = CreateOutputRedirectFile(argv[k+1]);
			}
		}

Select allOpen in new window

i still can't solve the problem as i tried it out it gave me this

prompt-> ls -l | grep test > file2
grep: >: No such file or directory
grep: file2: No such file or directory

means that it doesn't recognize the > and is unable to call the respective function???

>> means that it doesn't recognize the > and is unable to call the respective function???

It means that you are passing the ">" and "file2" as parameters when you call exec for the grep command.

You should NOT send the ">" and "file2" as parameters to the exec call. Only "grep" and "test".

In other words, make sure that you don't put ">" and "file2" in the argv array.

hmmm i'm comparing whether it is a < with this line
   >> if (strcmp(argv[k],input) == 0)
and if yes...then i'll pass the next argument (which is "file2" since i used argv[k+1]) to CreateOutputRedirectFile and there will create a "file2" if it doesn't exists and then return a value back?
or did i misunderstand anything?
   >>      w = CreateOutputRedirectFile(argv[k+1]);

isn't the argv in this line means the filename to be created?
   >>fdo = open(argv, O_CREAT | O_RDWR | O_TRUNC)

oops wait...so do you meant that if i found a > and so after i call the CreateOutputRedirectFile i have to delete the elements in argv [k] and also argv[k+1]??

>>In other words, make sure that you don't put ">" and "file2" in the argv array.
so do you think is it appropriate if i replace the > and file2 with null (k is the position for >)
            argv[k] = '\0';
            argv[k+1] = '\0';

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int *array2[SIZE];		// create an array of int * 
	int k = 0;
	int m = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
		}

		for(k = 0; k < m; k++)
		{
			if (strcmp(argv[k],input) == 0)
			{
				r = CreateInputRedirectFile(argv[k+1]);
				argv[k] = '\0';
				argv[k+1] = '\0';
			}
			else if (strcmp(argv[k],output) == 0)
			{
				w = CreateOutputRedirectFile(argv[k+1]);
				argv[k] = '\0';
				argv[k+1] = '\0';
			}
		}

		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int CreateInputRedirectFile(char *argv)
{
	int fdi;
	if (fdi = open(argv, O_RDONLY) < 0)	// Open the input files
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argv)
{
	int fdo;
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;
	if (fdo = open(argv, O_CREAT | O_RDWR | O_TRUNC, mode) < 0)	// Open the output files
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

hmmm even after i remove the elements  ">" and "file2" in the argv array but it still give me the same output:( sorry again for asking so many questions...

i successfully got this thing run... but when i check cat file....nothing is printed out... this means that the output of grep test wasn't written into file ryte...why is it like this?

prompt-> ls -l | grep test > file

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int *array2[SIZE];		// create an array of int * 
	int k = 0;
	int m = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;			
			// store each arguments into the array
			argument = strtok(NULL," \t\n");
			k++;
			m++;
		}

		for(k = 0; k < m; k++)
		{
			if (strcmp(argv[k],input) == 0)
			{
				r = CreateInputRedirectFile(argv[k+1]);
				argv[k] = '\0';
				argv[k+1] = '\0';
			}
			else if (strcmp(argv[k],output) == 0)
			{
				w = CreateOutputRedirectFile(argv[k+1]);
				argv[k] = '\0';
				argv[k+1] = '\0';
			}
		}

		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int CreateInputRedirectFile(char *argv)
{
	int fdi;
	if (fdi = open(argv, O_RDONLY) < 0)	// Open the input files
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argv)
{
	int fdo;
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;
	if (fdo = open(argv, O_CREAT | O_RDWR | O_TRUNC, mode) < 0)	// Open the output files
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

>> so do you meant that if i found a > and so after i call the CreateOutputRedirectFile i have to delete the elements in argv [k] and also argv[k+1]??

Yes. You cannot pass them to the exec function !

>> so do you think is it appropriate if i replace the > and file2 with null (k is the position for >)
>>             argv[k] = '\0';
>>             argv[k+1] = '\0';

In some cases that will work, but generally it's not as simple as that. The problem is that a NULL in the argv array will make the exec function ignore any arguments that might come after it. So if there are still valid arguments after argv[k], they will be ignored.

Note also that '\0' is a char - you probably wanted to use NULL.


A better way to go about this, is to simply not place them in the argv array. You could achieve that, by checking for < and > inside the while ((char *)argument != NULL) loop.



And finally : be careful with operator precedence :

>>       if (fdi = open(argv, O_RDONLY) < 0)

the < operator has a higher precedence than the = operator. So what you're storing in fdi is NOT the file descriptor, but the result of the comparison.
Use ()'s to force the assignment to be done first.
Or maybe better : do the assignment before the if.

hmm so i did the if else statement(check for the existence of > and <) inside the while loop and change some of the stuffs that you mentioned but this time the output is abit weird hmm...it seems like not writing anything to the file?

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int *array2[SIZE];		// create an array of int * 
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			argv[k] = (char *)argument;		// store each arguments into the array
			if (strcmp(argv[k],input) == 0)		// Check if the '<' exists in the argv array
			{
				r = CreateInputRedirectFile(argv[k+1]);	// Return a file descriptor from CreateInputRedirectFile
				argv[k] = NULL;				// Remove '<' from the argv array
				argv[k+1] = NULL;			// Remove 'filename' from the argv array
			}
			else if (strcmp(argv[k],output) == 0)	// Check if the '>' exist in the argv array
			{
				w = CreateOutputRedirectFile(argv[k+1]); // Return a file descriptor from CreateOutputRedirectFile
				argv[k] = NULL;				 // Remove '>' from the argv array
				argv[k+1] = NULL;			 // Remove 'filename' from the argv array
			}
			argument = strtok(NULL," \t\n");	// tokenize the command into arguments
			k++;					// Increment k
		}

		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int CreateInputRedirectFile(char *argv)
{
	int fdi;	// file descriptor
	fdi = open(argv, O_RDONLY);	// Open the input files
	if (fdi < 0)
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argv)
{
	int fdo;	// file descriptor
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;			// permission mode for the created file
	fdo = open(argv, O_WRONLY | O_TRUNC | O_CREAT, mode); 	// Open the output files
	if (fdo < 0)	
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

prompt-> ls -l | grep test > file
-rw-r--r-- 1 cpfoo ugrad  6951 2011-04-22 05:31 test.c <-- why it prints this line, it shouldn't prints this
                                                                                         line whereas it should be writing it to file ryte?

prompt-> ls -l file
-rwx------ 1 cpfoo ugrad 0 2011-04-21 17:16 file <-- and while i check the file is 0 bytes means nothing
                                                                                written to it but the mode is correct?

and yeah i  have a question...if let's say before the execute func being call, i have r and w returned from the CreatePipe func and when it comes to the execute func, if it finds any < or > , i know the r or w will be written with the new return value...so it'll like wipe off the previous return value of the CraetePipe func right, does this matter?

or this case will nvr happen bcos > or < will only appear either in the first command or at the last command?

>> hmm so i did the if else statement(check for the existence of > and <) inside the while loop and change some of the stuffs that you mentioned but this time the output is abit weird hmm...

Think very carefully about what you're doing. There are a few things going wrong :

(a) you're using argv[k + 1] before something has been placed there.

(b) you're still placing the ">", "<" and filenames in the argv array.


>> i know the r or w will be written with the new return value...so it'll like wipe off the previous return value of the CraetePipe func right,

Indeed.

>> does this matter?

That depends on whether you want to give priority to the pipe or to the file redirection.



>> bcos > or < will only appear either in the first command or at the last command?

To make it useful they SHOULD only appear there. However, they COULD appear elsewhere too - making the command less useful.

i repeatedly check over and over again... and make sure everything goes accordingly...but the output is still the same?
this time i'll have the first argument...and check whether it is > or <, if is not then go to else and store into argv array.... when come to the 3rd argument if is a > then i'll do strtok again to get the next argument and send it to the CreateOutputRedirectFile (as the parameter) and then return a file descriptor back, out of the if loop and do strtok again??

argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			if (strcmp((char *)argument,input) == 0)		// Check if the '<' exists in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				r = CreateInputRedirectFile(argument);	// Return a file descriptor from CreateInputRedirectFile
			}
			else if (strcmp((char *)argument,output) == 0)	// Check if the '>' exist in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				w = CreateOutputRedirectFile(argument);  // Return a file descriptor from CreateOutputRedirectFile
			}
			else
			{
				argv[k] = (char *)argument;		// store each arguments into the array
				k++;					// Increment k
			}
			argument = strtok(NULL," \t\n");	// tokenize the command into arguments
		}

Select allOpen in new window

>> i repeatedly check over and over again... and make sure everything goes accordingly...

That looks ok.


>> but the output is still the same?

Did you also fix the operator precedence issue I pointed out earlier ?

yes i did it in this way>>

int CreateInputRedirectFile(char *argument)
{
	int fdi;	// file descriptor
	fdi = open(argument, O_RDONLY);	// Open the input files
	if (fdi < 0)
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argument)
{
	int fdo;	// file descriptor
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;			// permission mode for the created file
	fdo = open(argument, O_WRONLY | O_TRUNC | O_CREAT, mode); 	// Open the output files
	if (fdo < 0)	
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

Select allOpen in new window

So what's the full code ? And what's the output you're getting ?

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

#define SIZE 80

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove new lines or tab, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = 0;		// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");		// tokenize the command line by |
		i++;				// Increment i
	}	
	*count = i;				// Count equals to i
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int *array2[SIZE];		// create an array of int * 
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			if (strcmp((char *)argument,input) == 0)	// Check if the '<' exists in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				r = CreateInputRedirectFile(argument);	// Return a file descriptor from CreateInputRedirectFile
			}
			else if (strcmp((char *)argument,output) == 0)	// Check if the '>' exist in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				w = CreateOutputRedirectFile(argument); // Return a file descriptor from CreateOutputRedirectFile
			}
			else
			{
				argv[k] = (char *)argument;		// store each arguments into the array
				k++;					// Increment k
			}
			argument = strtok(NULL," \t\n");	// tokenize the command into arguments
		}

		if (r != -1)		// if fds[0] is valid
		{
			close(w);	// Not going to write in this child process, so we can close this end of pipe
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
			close(r); 	// Not going to write in this child process, so we can close this end of pipe
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);					// Create a pipe

	return (int *)fds;				// Return the two file descriptors
}

int CreateInputRedirectFile(char *argument)
{
	int fdi;	// file descriptor
	fdi = open(argument, O_RDONLY);	// Open the input files
	if (fdi < 0)
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argument)
{
	int fdo;	// file descriptor
	mode_t mode = S_IRUSR | S_IWUSR | S_IXUSR;			// permission mode for the created file
	fdo = open(argument, O_WRONLY | O_TRUNC | O_CREAT, mode); 	// Open the output files
	if (fdo < 0)	
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int * 
	int invalid = -1; 

	printf("prompt-> ");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{
		if(strcmp(buf, "\n")!=0)  // user command line
		{
			memset(temp,0,sizeof(temp));		// Remove the elements in temp array
			memset(array,0,sizeof(array));		// Remove the elements in array 
			int count = 0;				// Initialize count to 0
			int j = 0;				// Initialize j to 0
			ParseInput(buf, temp, &count);		// Parse the user input line
			
			if (count == 1)				// If count is 1 (only one command)
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}

        	}
		else 
			printf("prompt-> ");

		memset(buf,0,sizeof(buf));		// Remove all the elements in buf
	}
	return 0;
}

Select allOpen in new window

prompt-> ls -l | grep test > file
-rw-r--r-- 1 cpfoo ugrad  6951 2011-04-22 05:31 test.c

prompt-> ls -l file
-rwx------ 1 cpfoo ugrad 0 2011-04-22 10:45 file

prompt-> exit

For starters, you need to be careful about which file descriptors you close ... Don't close the wrong ones (at the wrong time).

Next, you'll also want to make sure that the argv array has a NULL at the end.

You'll also have to make sure to place copies of the arguments in the argv array - ie. not just pointers into the original command, because that'll get overwritten on the next iteration !


That's a few of the problems that a quick look over the code turned up. If fixing those isn't sufficient to solve the problem, you'll have to do some debugging. Try figuring out what goes wrong where by carefully observing the code while it executes (either by running it in a debugger, or by adding printfs in strategic locations). And once you've found the culprit, figure out how to fix it.

Get used to that, because it's a big part of development ;)

i made some changes...and discovered smt weird...
here're the few changes that i've made
i close the file descriptor outside the 2 if's loop.. and just to make sure that the last element of the argv array is NULL i added this argv[k] = NULL; i know is not appropriate to put it like that but just for testing...

argv[k] = NULL;

		if (r != -1)		// if fds[0] is valid
		{
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}
		if (w != -1)		// if fds[1] is valid
		{
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		close(r); 	// Not going to write in this child process, so we can close this end of pipe
		close(w);	// Not going to write in this child process, so we can close this end of pipe

Select allOpen in new window

however... the output is a little different... it works while the command doesn't have a pipe... but it doesn't work when the command has a pipe in between... is this means that the value in w didn't change??

prompt-> ls -l > file

prompt-> ls -l file
-rw-r----- 1 cpfoo ugrad 514 2011-04-22 18:00 file

prompt-> ls -l | grep test > file2

prompt-> ls -l file2
-rw-r----- 1 cpfoo ugrad 0 2011-04-22 18:00 file2

>>You'll also have to make sure to place copies of the arguments in the argv array - ie. not just pointers into the original command, because that'll get overwritten on the next iteration !

will there be any difference if i put this
argv[k] = argument;  //the argument  >> instead of >>      argv[k] = (char *)argument;  // pointer to argument        

i tried to add some printf at some of the place and this is the output

prompt-> ls -l | grep test > file
ls
-l
ls -l (null) (null) (null)
grep
test
write successful
write to file
grep test (null) (null) (null) <-- this proves that there's a null at the end of the argv array

prompt-> ls -l file
ls
-l
file
ls -l file (null) (null)
-rw-r----- 1 cpfoo ugrad 0 2011-04-23 09:53 file <-- nothing is written 0 bytes??

prompt-> ls -l > file <-- without | in btw commands
ls
-l
write successful
write to file
ls -l (null) (null) (null)

prompt-> ls -l file
ls
-l
file
ls -l file (null) (null)
-rw-r----- 1 cpfoo ugrad 519 2011-04-23 09:53 file <--ls -l is written into file with 519 bytes???

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

#define SIZE 80

/********************** Parse the Command Line by | and store in temp[i] ***********************************/
void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove new lines or tab, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = '\0';	// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");		// tokenize the command line by |
		i++;				// Increment i
	}	
	*count = i;				// Count equals to i
}

/******************** Parse the Command into arguments and exec the arguments **************************/
void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int *array2[SIZE];		// create an array of int * 
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		memset(argv,0,sizeof(argv));		// Remove the elements in argv array
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			if (strcmp((char *)argument,input) == 0)	// Check if the '<' exists in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				r = CreateInputRedirectFile(argument);	// Return a file descriptor from CreateInputRedirectFile
			}
			else if (strcmp((char *)argument,output) == 0)	// Check if the '>' exist in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				w = CreateOutputRedirectFile(argument); // Return a file descriptor from CreateOutputRedirectFile
				printf("write to file\n");
			}
			else
			{
				printf("%s\n",argument);
				argv[k] = argument;		// store each arguments into the array
				k++;					// Increment k

			}
			argument = strtok(NULL," \t\n");	// tokenize the command into arguments
		}

		printf("%s %s %s %s %s\n", argv[0], argv[1], argv[2], argv[3], argv[4]);

		if (r != -1)		// if fds[0] is valid
		{
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}

		if (w != -1)		// if fds[1] is valid
		{
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		close(r); 	// Not going to write in this child process, so we can close this end of pipe
		close(w);	// Not going to write in this child process, so we can close this end of pipe

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

/**************** Create a Pipe function if there exists a pipe in the command line ****************************/

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);					// Create a pipe

	return (int *)fds;				// Return the two file descriptors
}

/**************** Create a Input Redirect function if there exists a < in the command line **********************/

int CreateInputRedirectFile(char *argument)
{
	int fdi;	// file descriptor
	fdi = open(argument, O_RDONLY);	// Open the input files
	if (fdi < 0)
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

/**************** Create a Output Redirect function if there exists a > in the command line *********************/

int CreateOutputRedirectFile(char *argument)
{
	int fdo;	// file descriptor
	mode_t mode = S_IRUSR | S_IRGRP | S_IWGRP | S_IWUSR;			// permission mode for the created file
	fdo = open(argument, O_CREAT | O_RDWR | O_TRUNC, mode); 	// Open the output files
	if (fdo < 0)	
		printf("Open file fail");		// Error

	printf("write successful\n");

	return fdo;				// Return a file descriptor
}

/********* Main function that pass a particular parameters from ParseInput function to Execute function ****************/

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int * 
	int invalid = -1; 

	printf("prompt-> ");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{
		if(strcmp(buf, "\n")!=0)  // user command line
		{
			memset(temp,0,sizeof(temp));		// Remove the elements in temp array
			memset(array,0,sizeof(array));		// Remove the elements in array 
			int count = 0;				// Initialize count to 0
			int j = 0;				// Initialize j to 0
			ParseInput(buf, temp, &count);		// Parse the user input line
			
			if (count == 1)				// If count is 1 (only one command)
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}

        	}
		else 
			printf("prompt-> ");

		memset(buf,0,sizeof(buf));		// Remove all the elements in buf
	}
	return 0;
}

Select allOpen in new window

I'm a bit curious why you felt the need to ask for additional help. I've been assisting you on this question for two weeks now, and when I don't respond within 12 hours, you ask for more assistance. I'm especially surprised since we agreed that you didn't have to open a new question, as long as you agreed to do more research for yourself.


>> i added this argv[k] = NULL; i know is not appropriate to put it like that but just for testing...

Why is that not appropriate ?


>> will there be any difference if i put this
>> argv[k] = argument;  //the argument  >> instead of >>      argv[k] = (char *)argument;  // pointer to argument        

No, there's no difference between those two.
When I mentioned copying, I mean actually making a copy of the string (strdup eg. could be useful).

can anyone explain why is this happen?
i added some printf into the main func

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int * 
	int invalid = -1; 

	printf("prompt-> ");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{
		if(strcmp(buf, "\n")!=0)  // user command line
		{
			memset(temp,0,sizeof(temp));		// Remove the elements in temp array
			memset(array,0,sizeof(array));		// Remove the elements in array 
			int count = 0;				// Initialize count to 0
			int j = 0;				// Initialize j to 0
			ParseInput(buf, temp, &count);		// Parse the user input line
			
			if (count == 1)				// If count is 1 (only one command)
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				printf("first command %d\n",write_fds);
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					printf("last command %d\n",read_fds);
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}

        	}
		else 
			printf("prompt-> ");

		memset(buf,0,sizeof(buf));		// Remove all the elements in buf
	}
	return 0;
}

Select allOpen in new window

prompt-> ls -l | grep test > file
piping successful
first command 4
last command 3      <--WHY IS THIS LINE APPEAR BEFORE THE FIRST COMMAND FOR
                                     EXECUTE FUNC
ls
-l
ls -l (null) (null) (null)
grep
test
write successful
write to file
grep test (null) (null) (null)

because i've been dragging this post for quite a time... and i'm still stuck... i've done a lot of research too on how to solve my prob but i still can't figure out where has gone wrong...and i really hope to get this done as soon as possible too tat's why i request for attention, hope you understand:)

>>When I mentioned copying, I mean actually making a copy of the string (strdup eg. could be useful).

hmm why do in need to make a copy of string into argv array?? if i confirm that the argv array has the all the arguments that is need, do i still need to make a copy into the argv array??

for example i have this line
            printf("%s %s %s %s %s\n", argv[0], argv[1], argv[2], argv[3], argv[4]);
output
          grep test (null) (null) (null)

>> tat's why i request for attention, hope you understand:)

No I don't, since you already had my attention. But let's leave it at that.


>> last command 3      <--WHY IS THIS LINE APPEAR BEFORE THE FIRST COMMAND FOR
>>                                      EXECUTE FUNC

Processes are executed in parallel - you cannot predict when which process will get a slice of execution time.


>> hmm why do in need to make a copy of string into argv array?? if i confirm that the argv array has the all the arguments that is need, do i still need to make a copy into the argv array??

If you think it's ok, then look for other possible causes of your issue :)

>>Processes are executed in parallel - you cannot predict when which process will get a slice of execution time.

so that means there's a possibility of argv array can have to 2 different elements at one time....hmmm since the processes are running in parallel... that's the reason why u want me to make a copy of the those string into argv array is it??
hmm so how should i use strdup ?? because when i check on the net, it shows smt like duplicating a string into another array of pointer...
argv[k] = strdup(argument); // is it just like this??? but what does strdup does??? does it just duplicate the
                                                 argument and put into the argv array?

seriously i can't think of any ways to check....because all the printf seems to go accordingly.... but the problem is the redirection works if the command is without the pipe...and doesn't work for commands with pipe in between....http:#35453042

can someone help me out to solve this problem....i got no idea what should i do now??

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

#define SIZE 80

void ParseInput(char *buf, char **temp, int *count)
{
	int i = 0;
	int nos, s, pos;
	char *command;
	char space[] = " \t";	// remove space or tab before the command, if any
	char nl[] = "\t\n";	// remove new lines or tab, if any

	command = strtok(buf,"|");		// tokenize the command line by |
	while(command != NULL)
	{
		nos = strspn (command, space); 	// return the number of spaces or tab before the command
		for (s = 0; s < nos; s++)
			++command;		// increment the pointer of command to remove the spaces or tabs, if any
		if (pos = strcspn (command, nl))// find if there's any new line or tab after the command
			command[pos] = '\0';	// replace it with a null, if any			
		for (;;)				// make a loop
		{
			int n = strlen (command) - 1;	// last character position in command
			if (command[n] == ' ')		// if there's a space 
				command[n] = '\0';	// makes the string shorter
			else 
				break;			// out of the loop, if any non-white space is found
		}			
		temp [i] = (char *)command;		// store each commands into the array
		command = strtok(NULL,"|");		// tokenize the command line by |
		i++;				// Increment i
	}	
	*count = i;				// Count equals to i
}

void execute(char *temp, int r, int w)
{
	pid_t pid;
	char *argument;
	char *argv[80]={0};
	char *input = "<";
	char *output = ">";
	int k = 0;
	int fds[2];
	pid = fork();

	if (pid == 0)	/* This is the child process */
	{
		memset(argv,0,sizeof(argv));		// Remove the elements in argv array
		argument = strtok((char *)temp," \t\n");	// tokenize the command into arguments
		while ((char *)argument != NULL)
		{
			if (strcmp((char *)argument,input) == 0)	// Check if the '<' exists in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				r = CreateInputRedirectFile(argument);	// Return a file descriptor from CreateInputRedirectFile
			}
			else if (strcmp((char *)argument,output) == 0)	// Check if the '>' exist in the argv array
			{
				argument = strtok(NULL," \t\n");	// tokenize the command into arguments
				w = CreateOutputRedirectFile(argument); // Return a file descriptor from CreateOutputRedirectFile
			}
			else
			{
				argv[k] = strdup(argument);		// store each arguments into the array
				k++;					// Increment k
			}
			argument = strtok(NULL," \t\n");	// tokenize the command into arguments
		}
		argv[k] = NULL;

		if (r != -1)		// if fds[0] is valid
		{
			dup2(r,0); 	// Reassign stdin to r = fds[0] end of pipe
		}

		if (w != -1)		// if fds[1] is valid
		{
		  	dup2(w,1); 	// Reassign stdout to w = fds[1] end of pipe
		}

		close(r); 	// Not going to write in this child process, so we can close this end of pipe
		close(w);	// Not going to write in this child process, so we can close this end of pipe

		execvp(argv[0],argv);		// execute the given command
		write(1,"exec fail\n",10);	// print this message if fails to execvp
		exit(1);
	}
	else if (pid < 0) 		// error
	{
		printf("fork failed");
		exit(1);
	}
}

int *CreatePipe()
{
	int *fds = (int *)calloc(2, sizeof(int));	// Dynamically allocated memory
	pipe(fds);					// Create a pipe

	return (int *)fds;				// Return the two file descriptors
}

int CreateInputRedirectFile(char *argument)
{
	int fdi;	// file descriptor
	fdi = open(argument, O_RDONLY);	// Open the input files
	if (fdi < 0)
		printf("Open read fail");		// Error

	return fdi;				// Return a file descriptor
}

int CreateOutputRedirectFile(char *argument)
{
	int fdo;	// file descriptor
	mode_t mode = S_IRUSR | S_IRGRP | S_IWGRP | S_IWUSR;			// permission mode for the created file
	fdo = open(argument, O_CREAT | O_RDWR | O_TRUNC, mode); 	// Open the output files
	if (fdo < 0)	
		printf("Open file fail");		// Error

	return fdo;				// Return a file descriptor
}

int main()
{
	int j = 0;
        char buf[80];   	// command line
        char *temp[80]={0};	// an array to store the commands
	int count;
	pid_t pid;
	int read_fds, write_fds;
	int *array[SIZE];		// create an array of int * 
	int invalid = -1; 

	printf("prompt-> ");
	while (strcmp(fgets((char *)buf, 80, stdin), "exit\n") !=0)  // user command line
	{
		if(strcmp(buf, "\n")!=0)  // user command line
		{
			memset(temp,0,sizeof(temp));		// Remove the elements in temp array
			memset(array,0,sizeof(array));		// Remove the elements in array 
			int count = 0;				// Initialize count to 0
			int j = 0;				// Initialize j to 0
			ParseInput(buf, temp, &count);		// Parse the user input line
			
			if (count == 1)				// If count is 1 (only one command)
			{
				execute(temp[j], invalid, invalid);	// call execute func
			}
			if (count > 1)				// for more than one command (with pipes)
			{
				array[j] = CreatePipe();	// store the return value of CreatePipe to array[j]
				write_fds = array[0][1];	// copy the write end of the pipe to write_fds
				execute(temp[j], invalid, write_fds);	// call the execute func and pass the end of pipe
				j++;					// increment of j
 
				for(j = 1; j < count - 1; j++)			// for each command
				{
					array[j] = CreatePipe();		// create another pipe
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					write_fds = array[j][1];		// copy the write end of current pipe
					execute(temp[j], read_fds, write_fds);	// call the execute func
				}
		
				if (j == count - 1)				// for the last command
				{
					read_fds = array[j-1][0];		// copy the read end of previous pipe
					execute(temp[j], read_fds, invalid);	// call the execute func
				}
			}

        	}
		else 
			printf("prompt-> ");

		memset(buf,0,sizeof(buf));		// Remove all the elements in buf
	}
	return 0;
}

Select allOpen in new window

prompt-> ls -l | grep test
-rw-r--r-- 1 cpfoo ugrad  6951 2011-04-22 05:31 test.c

prompt-> ls -l > file

prompt-> ls -l file
-rw-r----- 1 cpfoo ugrad 517 2011-04-24 20:38 file
 
prompt-> ls -l | grep test > file2 <-- NOT WORKING?!?!?

prompt-> ls -l file2
-rw-r----- 1 cpfoo ugrad 0 2011-04-24 20:38 file2

>> that's the reason why u want me to make a copy of the those string into argv array is it??

I'm just offering you food for thought ... so you can make up your own mind about what's necessary and what isn't ;)


>> but the problem is the redirection works if the command is without the pipe...and doesn't work for commands with pipe in between....

In http:#35449571, I gave you the advice to be very careful about closing file descriptors when needed. You have made modifications in the child process, but you have not changed the parent process accordingly.
Make sure that you close unneeded file descriptors at the right moment, and that you don't close file descriptors that are still needed.

sorry for the late reply due to my finals...but i solved the problem:) thanks.