I am working on a simple C program that recursively creates children and uses them to sum all the numbers in a file, depending on user input. There are three predetermined file sizes that the user can chose from, as well as three set amounts of children that can be generated. In theory, there could be any number of children or any size file, but for the sake of simplicity there are only 3 here.
The problem I'm running into is, no matter which file I use, the only time the sum is correct is when the program uses only 1 child. With other amounts of children, such as 4, the number is close, but not quite right. Can someone offer me any insight as to what is causing this issue?
Here is the section of code I think is problematic:
// C program to demonstrate use of fork() with pipe()
// By: Maxwell Wendlandt
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <time.h>
#include <sys/wait.h>
#include <unistd.h>
int main()
{
FILE *file;
int numForks;
// initialize pipes for up to 4 children
int fd[4][2];
// initialize up to 4 processes
pid_t pid[4];
int total = 0;
int finalResult = 0;
char fileName[10] = "";
int fileNum;
int numLines;
// ask which file to scan
printf("Enter file number 1 (1000 nums), 2 (10000 nums) or 3 (100000 nums):\n");
scanf("%i", &fileNum);
// chose the file
switch(fileNum)
{
case 1 :
printf("File 1 selected.\n");
strcpy(fileName, "file1.dat");
numLines = 1000;
break;
case 2 :
printf("File 2 selected.\n");
strcpy(fileName, "file2.dat");
numLines = 10000;
break;
case 3 :
printf("File 3 selected.\n");
strcpy(fileName, "file3.dat");
numLines = 100000;
break;
default :
printf("Enter a valid file number next time.\n");
return 0;
}
// ask how many children (forks)
printf("Do you want 1, 2 or 4 child processes?\n");
scanf("%i", &numForks);
for (int i = 0; i < numForks + 1; i++)
{
if (pipe(fd[i]) == -1)
{
printf("Error with creating pipe.\n");
return 1;
}
}
for(int i = 0; i < numForks; i++)
{
pid[i] = fork();
if(pid[i] == -1)
{
printf("Error creating child.\n");
return 1;
}
if(pid[i] == 0)
{
// children
int sum = 0, num = 0;
int start, end;
file = fopen(fileName, "r");
start = i * (numLines / numForks);
printf("start: %i\n", start);
end = ((i + 1) * (numLines / numForks));
printf("end: %i\n", end);
fseek(file, (start * 4), SEEK_SET);
for(int i = start; i < end; i++)
{
fscanf(file, "%d", &num);
printf("num on line %d is: %d\n", i + 1, num);
sum += num;
}
printf("sum in child: %d\n", sum);
write(fd[i][1], &sum, sizeof(sum));
close(fd[i][1]);
return 0;
}
}
// parent
for(int i = 0; i < numForks; i++)
{
read(fd[i][0], &total, sizeof(total));
close(fd[i][0]);
finalResult += total;
}
printf("The grand total: %i\n", finalResult);
for(int i = 0; i < numForks; i++)
{
wait(NULL);
}
return 0;
}
Thanks in advance!
each line of the file has one 3 digit number on it. So a 1000 number file has 1000 lines.
This means each line consists of four five bytes - the three digits, the carriage return, and the newline character. e.g., 123\r\n. The off-by-two error here
fseek(file, (start * 3), SEEK_SET);
will cause each seek to drift, and each child will read from an earlier position than they should. If each line is five bytes, this should be start * 5.
Aside: I would hazard a guess the numbers in your files are padded with zeroes (see the generation example below).
If so, the fscanf specifier %i may not be desirable, as it acts as strtol with a base of 0, meaning the numbers base is determined by its first characters.
This may lead to confusing results when zero padded numbers are parsed as octal. For example:
004 - octal, value 4.040 - octal, value 32.400 - decimal, value 400.009 - octal, invalid value (0).011 - octal, value 9.%d will parse the inputs as base-10 numbers.
This has a few problems.
printf("Do you want 1, 2 or 4 child processes?\n");
scanf("%i", &numForks);
for (int i = 0; i < numForks + 1; i++) {
if (pipe(fd[i]) == -1)
/* ... */
i < numForks + 1 is an off-by-one error. The user can also enter an arbitrary number.This will invoke Undefined Behaviour if fd is accessed via an out-of-bounds index.
In general, you should be checking the return values of more functions, such as scanf, fscanf, fseek, write, and read, to ensure you are working with valid data.
Prefer perror and fprintf(stderr, ...) to print useful error messages to the correct stream.
A very cursory refactoring:
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <wait.h>
int main(void)
{
int numForks;
// initialize pipes for up to 4 children
int fd[4][2];
// initialize up to 4 processes
pid_t pid[4];
int total = 0;
int finalResult = 0;
char fileName[10] = "";
int fileNum;
int numLines;
printf("Enter file number 1 (1000 nums), 2 (10000 nums) or 3 (100000 nums):\n");
if (1 != scanf("%d", &fileNum)) {
fprintf(stderr, "Invalid number of files.\n");
return 1;
}
switch (fileNum) {
case 1:
printf("File 1 selected.\n");
strcpy(fileName, "file1.dat");
numLines = 1000;
break;
case 2:
printf("File 2 selected.\n");
strcpy(fileName, "file2.dat");
numLines = 10000;
break;
case 3:
printf("File 3 selected.\n");
strcpy(fileName, "file3.dat");
numLines = 100000;
break;
default:
printf("Enter a valid file number next time.\n");
return 0;
}
printf("Do you want 1, 2 or 4 child processes?\n");
if (1 != scanf("%d", &numForks) || 1 > numForks || numForks > 4 || numForks == 3) {
fprintf(stderr, "Invalid number of child processes.\n");
return 1;
}
for (int i = 0; i < numForks; i++) {
if (pipe(fd[i]) == -1) {
perror("pipe");
return 1;
}
}
for (int i = 0; i < numForks; i++) {
pid[i] = fork();
if (pid[i] == -1) {
perror("fork");
return 1;
}
// children
if (pid[i] == 0) {
int sum = 0, num = 0;
int start, end;
FILE *file = fopen(fileName, "r");
if (!file) {
fprintf(stderr, "Child %d failed to open ", i + 1);
perror(fileName);
return 1;
}
start = i * (numLines / numForks);
end = ((i + 1) * (numLines / numForks));
printf("start: %d\nend: %d\n", start, end);
if (-1 == fseek(file, (start * 4), SEEK_SET)) {
perror("fseek");
return 1;
}
for (int i = start; i < end; i++)
if (1 == fscanf(file, "%d", &num))
sum += num;
printf("sum in child: %d\n", sum);
write(fd[i][1], &sum, sizeof sum);
close(fd[i][1]);
return 0;
}
}
// parent
for (int i = 0; i < numForks; i++) {
if (sizeof total == read(fd[i][0], &total, sizeof total))
finalResult += total;
close(fd[i][0]);
}
for (int i = 0; i < numForks; i++)
wait(NULL);
printf("The grand total: %d\n", finalResult);
}
The code used to generate files to test with (./gen 1000 > file1.dat):
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
int main(int argc, char **argv)
{
int i = 0;
if (argc > 1)
i = atoi(argv[1]);
srand((unsigned) time(NULL));
while (i-- > 0)
printf("%03d\n", rand() % 1000);
}
And a sanity checker (./sanity-check < file1.dat):
#include <stdio.h>
int main(void)
{
int sum = 0, num;
while (1 == scanf("%d", &num))
sum += num;
printf("%d\n", sum);
}