A few issues:
- The children just
read
forever. They never check for EOF.
- Parent
wait
in wrong place. Should be at end after closing all write ends of pipes.
- Child must close all write ends of pipes before starting its loop and not just its own.
- Child must close all read ends of pipes except its own.
- Otherwise, each child interferes with a given child seeing EOF on its pipe.
- Parent never detects EOF and never terminates its loop.
- Child can not rely on receiving an EOS (0x00) terminator in a single
read
because pipes can return short reads [or short writes].
- i.e. Child should
fwrite
to stdout
instead of doing printf
of received buffer.
Here is the refactored code. It is annotated with bugs and fixes:
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <sys/wait.h>
enum {
R = 0,
W = 1
};
char query[100];
int
main(int argc, char const *argv[])
{
// Création de pipes anonymes (un pour chaque processus)
int fd[4][2];
pid_t pid;
for (int i = 0; i < 4; i++) {
pipe(fd[i]);
printf("PIPE %d R=%d W=%d\n",i,fd[i][R],fd[i][W]);
}
pid_t fils_pid[4];
// Fork 4 processus fils
for (int i = 0; i < 4; i++) {
if ((fils_pid[i] = fork()) == 0) {
// NOTE/BUG: _each_ child must close _all_ write ends
// NOTE/BUG: _each_ child must close _all_ read ends _except_ its own
#if BUG
close(fd[i][W]); // Ferme les descripteurs d'écriture
#else
for (int j = 0; j < 4; ++j) {
close(fd[j][W]);
if (j != i)
close(fd[j][R]);
}
#endif
// NOTE/BUG: we can't guarantee an EOS terminator because of pipe short read
// NOTE/BUG: we never stop on EOF
#if BUG
while (1) {
read(fd[i][R], query, 100);
printf("Query: %s", query);
}
#else
while (1) {
ssize_t rlen = read(fd[i][R], query, sizeof(query));
printf("Query/%d/%zd: ",i,rlen);
if (rlen <= 0)
break;
fwrite(query,1,rlen,stdout);
}
#endif
close(fd[i][R]);
exit(EXIT_SUCCESS);
}
#if 1
printf("main: pid[%d]=%d\n",i,fils_pid[i]);
#endif
}
// children never reach here
for (int i = 0; i < 4; i++) {
printf("main: RCLOSE i=%d\n",i);
close(fd[i][R]);
}
while (1) {
int i = rand() % 4;
printf("Enter a query: ");
fflush(stdout);
// NOTE: bug we don't stop on EOF
#if BUG
fgets(query, 100, stdin);
#else
if (fgets(query, sizeof(query), stdin) == NULL) {
printf("eof ...\n");
break;
}
#endif
// NOTE/FIX: stop sending to children if we get a blank line
#if ! BUG
if (query[0] == '\n') {
printf("stop ...\n");
break;
}
#endif
#if BUG
write(fd[i][W], query, strlen(query) + 1);
#else
write(fd[i][W], query, strlen(query));
#endif
#if BUG
wait(NULL);
#endif
}
// NOTE/BUG: we should close write ends of _all_ pipes
#if 0
close(fd[t][W]);
#else
for (int i = 0; i < 4; i++) {
printf("main: WCLOSE i=%d\n",i);
close(fd[i][W]);
}
#endif
// NOTE/FIX: correct place to wait for children (_after_ closing pipes)
#if ! BUG
while (1) {
pid = wait(NULL);
printf("wait: pid=%d\n",pid);
if (pid < 0)
break;
}
#endif
printf("Bye bye!\n");
return 0;
}
In the above code, I've used cpp
conditionals to denote old vs. new code:
#if 0
// old code
#else
// new code
#endif
#if 1
// new code
#endif
I've also used #if BUG
and #if ! BUG
for original code and bug fixes.
Note: this can be cleaned up by running the file through unifdef -k