Pipe & Fork code - need help

SummerBulb

Hi there. I am required to write an application in C for Unix that does the following: The program will create 3 child processes which read from 3 different files and write to the same pipe in the parent process. Each child should wait a random amount of time (3 -10 seconds) between writing each 50 characters. The father should read from the pipe and write everything he gets (from all 3 files) into one new file. Here is my Code:

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

int main(int argc, char* argv[])
{
if (argc != 5)
{
printf("Error!\nUsage: tar1.exe <input file 1> <input file 2> <input file 3> <output file>\n");
exit(0);
}

int fd\[2\];
int fileOpen;
int pids\[3\];
int i;
char buf\[51\];
pipe(fd);	

close(fd\[1\]); //father only reads from pipe

for (i=1; i < 4; i++)
{
	switch ((pids\[i\] = fork()))
	{
		case -1:
			printf("Fork error");
			break;
		case 0:
			close(fd\[0\]); //child only writes to pipe
			printf("Child %d created. File: %s\\n",i ,argv\[i\]);
			if ((fileOpen = open(argv\[i\],O\_RDONLY)) == -1)
			{
				printf("File %s does not exist.\\n",argv\[i\]);
				exit(0);
			}
			printf("Child %d writing to pipe\\n",i);
			while (read(fileOpen,buf,50) > 0)
			{
				buf\[50\] = NULL;
				printf("Child %d. Text: %s",i,buf);
				write(fd\[1\], buf, 50);
				sleep(rand()%8+3);
			}
			close(fd\[1\]); //child done writing
			close(fileOpen);
			printf("Child %d done writing to pipe\\n",i);
			exit(0);
			break;
		default:
			break;
	}	
}

wait();
wait();
wait();

if ((fileOpen = open(argv\[4\],O\_WRONLY)) == -1)
{
	printf("File %s does not exist.\\n",argv\[i\]);
	exit(0);
}
printf("Father reading from pipe.\\n");
//sleep(rand()%8+3);
while (read(fd\[0\],buf,50) != 0)
{
	printf("Read line...\\n");
	buf\[50\] = NULL;
	printf(buf);
	write(fileOpen, buf, 50);
	//sleep(rand()%8+3);
}
close(fd\[0\]);
close(fileOpen);
printf("Done reading!\\n");
	
return 0;

}

Running the application results with lots of rubbish in the output file. Help will be highly appreciated. Thanks!

CPallini

SummerBulb wrote:

while (read(fd[0],buf,50) != 0) { printf("Read line...\n"); buf[50] = NULL; printf(buf); write(fileOpen, buf, 50)

You're assuming the read function return either 0 or 50. This is a wrong assumption. BTW Haven't you any synchronization requirement (for instance, it is ok if in the output file the content of the three input files is scrambled?)? :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

SummerBulb

CPallini wrote:

SummerBulb wrote: while (read(fd[0],buf,50) != 0) { printf("Read line...\n"); buf[50] = NULL; printf(buf); write(fileOpen, buf, 50) You're assuming the read function return either 0 or 50. This is a wrong assumption.

I changes the while condition to

while (read(fd[0],buf,50) > 0)

but that is also wrong, of course. If i'm not mistaken, im getting a -1 return for an unknown reason. Why can't the father read from the pipe?

CPallini wrote:

BTW Haven't you any synchronization requirement (for instance, it is ok if in the output file the content of the three input files is scrambled?)?

I tried very hard to find the sync demand int the text, but i couldn't find it... :) Thanks!

CPallini

You may have, at least, two problems:

read return value is -1.
read return value is positive, but less than 50.

When read returns -1, you've to check errno to understand what happened (in any case you must NOT write (garbage) values to the output file. When read returns a positive value less than 50, you should copy just the read bytes to the output file. For instance

while ( ! finished )
{
ssize nread = read(fd[0],buf,50);
if ( nread == -1 )
{
// handle error
}
else if (nread > 0)
{
printf("Read %d characters\n", nread);
buf[nread]='\0';
printf(buf);
write(fileOpen, buf, nread);
}
}

where finished is a termination condition (timeout?) that you possibly know better than me.

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

markkuk

SummerBulb wrote:

Here is my Code:

int pids[3];
int i;
//...
for (i=1; i < 4; i++)
{
switch ((pids[i] = fork()))

You forgot that C arrays are zero-based.

SummerBulb

:^) Oh, oops. How did that happen? Anyway, that array is not used later, and iv'e removed it from the code, so that isn't the problem. Thanks anyway.

SummerBulb

CPallini wrote:

For instance while ( ! finished ) { ssize nread = read(fd[0],buf,50); if ( nread == -1 ) { // handle error } else if (nread > 0) { printf("Read %d characters\n", nread); buf[nread]='\0'; printf(buf); write(fileOpen, buf, nread); } }

I already tried rewriting that part. When i get nread=-1 i print a message, just for the debug. The result of that change is an endless loop of messages. For some reason, when it gets to the father, the pipe is empty, or some other pipe error. The father just can't read any info from the pipe. Man, this is frustrating!

CPallini wrote:

where finished is a termination condition (timeout?) that you possibly know better than me.

The thing is - the end condition is an empty pipe...

CPallini

Try:

#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
int main(int argc, char* argv[])
{
if (argc != 5)
{
printf("Error!\nUsage: %s <input file 1> <input file 2> <input file 3> <output file>\n", argv[0]);
return -1;
}

int fd\[2\];
int fileOpen;
int i;
char buf\[51\];
pipe(fd);	
ssize\_t nread;
int count = 0;

for (i=1; i < 4; i++)
{
	switch (fork())
	{
	case -1:
		printf("Error: fork failure.\\n");
		return -2;
	case 0:
		close(fd\[0\]); //child only writes to pipe
		printf("Child %d created. File: %s\\n", i ,argv\[i\]);
		if ((fileOpen = open(argv\[i\], O\_RDONLY)) == -1)
		{
			printf("File %s does not exist.\\n", argv\[i\]);
			return -3;
		}
		printf("Child %d writing to pipe\\n",i);
		while ((nread = read(fileOpen,buf,50)) > 0)
		{
			buf\[nread\] = '\\0';
			printf("Child %d. Text: %s\\n", i, buf);
			write(fd\[1\], buf, nread);
			usleep(25000\*(rand()%8+3));
		}
		close(fd\[1\]); //child done writing
		close(fileOpen);
		printf("Child %d done writing to pipe\\n",i);
		return 0;
	default:
		break;
	}
}
// father	
close(fd\[1\]); //father only reads from pipe
if ((fileOpen = open(argv\[4\],O\_WRONLY|O\_CREAT)) == -1)
{
	printf("Unable to open file %s.\\n", argv\[4\]);
	return -4;
}
printf("Father reading from pipe.\\n");
while (count < 1000)
{
	if ((nread = read(fd\[0\],buf, 50)) > 0)
	{
		int i;
		printf("Father read line...\\n");
		buf\[nread\] = '\\0';
		printf("\\n%s\\n",buf);
		write(fileOpen, buf, nread);
	}
	else
	{
		printf("Father waiting for data...\\n");
		usleep(10000);
	}
	count++;
}
close(fd\[0\]);
close(fileOpen);
printf("Father Done reading!\\n");
	
return 0;

}

:)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

SummerBulb

Just to let you know - the code works perfectly. Thank you very much!

CPallini

You are welcome. Thanks for the feedback. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]