Parent process can't read all messages from 4 different pipes in C

Question

Finally after reviewing and asking questions here, I was able to write this code which generates 4 child processes and communicate via 4 different pipes. Each of the 4 child processes writes into a single pipe (fd_log), but when the parent process reads from the pipe it reads only the first message from process A, the first message from process C, all messages from process B and none from process D. This is the code:

int main(int argc, char *argv[]) {

    printf("\nWritten by Nawar Youssef\n");
    int i, x=0, fd_log[2], fd_A_B[2], fd_B_C[2], fd_B_D[2], pipe_size=500;
    char ch, msg_from_A[pipe_size], msg_to_B[pipe_size], msg_to_log[pipe_size],
        msg_to_C[pipe_size], msg_to_D[pipe_size], msg_B_C[pipe_size], msg_B_D[pipe_size],
        msg_from_log[pipe_size], temp_char[pipe_size], msg_C_log[pipe_size], msg_D_log[pipe_size];

    pipe(fd_log);
    pipe(fd_A_B);
    pipe(fd_B_C);
    pipe(fd_B_D);

    if (fork()==0) { //child A
        for (i=0; i < 10; i++) {
            x = (rand() % 2);
            if (x == 1)
                ch='C';
            else
                ch='D';

            //write records to A-B pipe
            close(fd_A_B[READ]);
            sprintf(msg_to_B, "%c %d", ch, i);
            write(fd_A_B[WRITE], msg_to_B, strlen(msg_to_B)+1);

            //write records to log pipe
            close(fd_log[READ]);
            sprintf(msg_to_log, "A sent to B: %c %d", ch, i);
            write(fd_log[WRITE], msg_to_log, strlen(msg_to_log)+1);
        }//for
        close(fd_A_B[WRITE]);
        close(fd_log[WRITE]);
        _exit(1);
    }//if

    if (fork()==0) { //child B
        //read A-B pipe
        close(fd_A_B[WRITE]);
        int n_bytes = read(fd_A_B[READ], msg_from_A, sizeof(msg_from_A));
        for(i=0; i < pipe_size; i++) {

            if ( msg_from_A[i] == 'C') {

                //write the message from B to C pipe
                sprintf(msg_to_C, "%c %c", msg_from_A[i], msg_from_A[i+2]);
                //printf("--%s\n", msg_to_C);
                close(fd_B_C[READ]);
                write(fd_B_C[WRITE], msg_to_C, strlen(msg_to_C)+1);

                //write C message to log pipe
                close(fd_log[READ]);
                sprintf(msg_to_log, "B sent to C: %s", msg_to_C);
                write(fd_log[WRITE], msg_to_log, strlen(msg_to_log)+1);
                sleep(1);

            }else if (msg_from_A[i] == 'D') {

                //write the message from B to D pipes
                sprintf(msg_to_D, "%c %c", msg_from_A[i], msg_from_A[i+2]);
                //printf("--%s\n", msg_to_D);
                close(fd_B_D[READ]);
                write(fd_B_D[WRITE], msg_to_D, strlen(msg_to_D)+1);

                //write D message to log pipe
                close(fd_log[READ]);
                sprintf(msg_to_log, "B sent to D: %s", msg_to_D);
                write(fd_log[WRITE], msg_to_log, strlen(msg_to_log)+1);
                sleep(5);

            }else
                continue;
        }//for
        close(fd_B_C[WRITE]); close(fd_B_D[WRITE]); close(fd_log[WRITE]);
        _exit(1); //process B
    }//if

    if (fork()==0) { //child C
        //read from B-C pipe
        close(fd_B_C[WRITE]);
        int n_bytes = read(fd_B_C[READ], msg_B_C, sizeof(msg_B_C));
        for (i=0; i < pipe_size; i++) {

            //write to log pipe
            if (msg_B_C[i] == 'C') {
                sprintf(msg_C_log, "%c %c", msg_B_C[i], msg_B_C[i+2]);
                close(fd_log[READ]);
                sprintf(msg_to_log, "C sent to log: %s", msg_C_log);
                write(fd_log[WRITE], msg_to_log, strlen(msg_to_log)+1);
            }else
                continue;
        }
        _exit(1); //process C
    }

    if (fork()==0) { //child D
        //read from fd_B_D
        close(fd_B_D[WRITE]);
        int n_bytes = read(fd_B_D[READ], msg_B_D, sizeof(msg_B_D));
        for (i=0; i < pipe_size; i++) {

            //write to log pipe
            if (msg_B_D[i] == 'D') {
                sprintf(msg_D_log, "%c %c", msg_B_D[i], msg_B_C[i+2]);
                close(fd_log[READ]);
                sprintf(msg_to_log, "D sent to log: %s", msg_D_log);
                write(fd_log[WRITE], msg_to_log, strlen(msg_to_log)+1);
            }
        }
        _exit(1);
    }//if

    //parent
    close(fd_log[WRITE]);
    int n_bytes;
    while( (n_bytes = read(fd_log[READ], msg_from_log, sizeof(msg_from_log)-1)) > 0){
        printf("Log pipe reads (%d) bytes: %s\n", n_bytes, msg_from_log);
        sleep(1);
    }
    close(fd_log[READ]);
    return (0);
}

When I look at the output, I know that all messages were read (see in the output the number of bytes read returned). So it looks like the problem is in displaying the messages. I have faced a similar problem with much simpler code and I solved it by making pipe_size larger. But here it didn't work.
This is the output number between (#) is the bytes read each time the while loops:

Written by Nawar Youssef

Log pipe reads (187) bytes: A sent to B: C 0

Log pipe reads (36) bytes: C sent to log: C 0

Log pipe reads (17) bytes: B sent to C: C 2

Log pipe reads (35) bytes: B sent to D: D 3

Log pipe reads (17) bytes: B sent to D: D 4

Log pipe reads (17) bytes: B sent to D: D 5

Log pipe reads (17) bytes: B sent to D: D 6

Log pipe reads (17) bytes: B sent to D: D 7

Log pipe reads (17) bytes: B sent to C: C 8

Log pipe reads (17) bytes: B sent to C: C 9

Answer 1

The code is a little confusing because you just decided to put everything in main(). A few auxiliary functions would be useful, but I'm not here to judge that.

A couple of more important mistakes:

• No error handling. Not one. read(2), write(2), close(2), pipe(2) and fork(2) can all return an error. You never check for that, and you should. If you did, you would see that close(2) is returning EBADF in most cases, because you call it inside a loop with the same file descriptor over and over. For example, look at the code for child A. It closes two pipes inside the loop. As soon as the loop hits the second iteration, close(2) starts returning an error because you already closed that descriptor.
• You also never close the pipes write channels in the parent. This will leave the other child processes alive and blocked in read(2) because there is still one active writer in the pipes (the parent) that could (but won't) write. You must close the pipes write ends in the parent. It is always a good idea to create the pipes in the parent immediately before forking the children that will use them, and close the unused channels in the parent immediately after forking. The same goes for the child processes.

• Each child should read the pipe inside a loop similar to what the parent does with the log. Otherwise, you will just read once and discard the other messages that are written in the meantime. You must keep reading until the pipe's write channel is closed by the other end (which happens when read(2) returns 0).

• Your loop conditions in the children to parse the message read from the pipe should be i+2 < nbytes, not i < pipe_size - the latter will go out of bounds when the message is smaller than pipe_size bytes (which is very likely). The update condition should also be i+= 3 since you consume 3 bytes per iteration.

• else continue; in the end of a loop is utterly useless.

• Please compile your code with warnings enabled. You would see that you have a variable temp_char that is never used.

• Child D should read the number from msg_B_D instead of msg_B_C. I believe this was a copy / paste error.

• You can't send the null terminator over the pipe. You are doing this, and because of that, you don't see every message that you sent to the log when you print it to output - remember that printf() only prints a string until it sees a terminating null character. Instead, I suggest that you send the messages to the log with a newline ready for the parent to print it without further worries.

Here's the code with all of these mistakes addressed. I commented out the sleeps to get the output quickly:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <unistd.h>
#include <fcntl.h>
#include <time.h>

#define READ 0
#define WRITE 1

int main(int argc, char *argv[]) {
    printf("\nWritten by Nawar Youssef\n");
    int i, x=0, fd_log[2], fd_A_B[2], fd_B_C[2], fd_B_D[2], pipe_size=500;
    char ch, msg_from_A[pipe_size], msg_to_B[pipe_size], msg_to_log[pipe_size],
        msg_to_C[pipe_size], msg_to_D[pipe_size], msg_B_C[pipe_size], msg_B_D[pipe_size],
        msg_from_log[pipe_size], msg_C_log[pipe_size], msg_D_log[pipe_size];

    if (pipe(fd_log) < 0) {
        fprintf(stderr, "%d: pipe(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pipe(fd_A_B) < 0) {
        fprintf(stderr, "%d: pipe(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    pid_t pid;

    if ((pid = fork()) < 0) {
        fprintf(stderr, "%d: fork(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pid == 0) {
        //child A
        for (i = 0; i < 10; i++) {
            x = (rand() % 2);
            if (x == 1)
                ch = 'C';
            else
                ch = 'D';

            sprintf(msg_to_B, "%c %d", ch, i);
            size_t to_write = strlen(msg_to_B);
            if (write(fd_A_B[WRITE], msg_to_B, to_write) < 0) {
                fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                exit(EXIT_FAILURE);
            }

            sprintf(msg_to_log, "A sent to B: %c %d\n", ch, i);
            to_write = strlen(msg_to_log);
            if (write(fd_log[WRITE], msg_to_log, to_write) < 0) {
                fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                exit(EXIT_FAILURE);
            }
        }

        exit(EXIT_SUCCESS);
    }

    if (close(fd_A_B[WRITE]) < 0) {
        fprintf(stderr, "%d: close(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pipe(fd_B_C) < 0) {
        fprintf(stderr, "%d: pipe(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pipe(fd_B_D) < 0) {
        fprintf(stderr, "%d: pipe(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if ((pid = fork()) < 0) {
        fprintf(stderr, "%d: fork(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pid == 0) {
        // Child B

        int n_bytes;
        while ((n_bytes = read(fd_A_B[READ], msg_from_A, sizeof(msg_from_A)-1)) > 0) {
            size_t to_write;

            for (i = 0; i+2 < n_bytes; i += 3) {
                if (msg_from_A[i] == 'C') {
                    sprintf(msg_to_C, "%c %c", msg_from_A[i], msg_from_A[i+2]);
                    to_write = strlen(msg_to_C);
                    if (write(fd_B_C[WRITE], msg_to_C, to_write) != to_write) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }

                    sprintf(msg_to_log, "B sent to C: %s\n", msg_to_C);
                    to_write = strlen(msg_to_log);
                    if (write(fd_log[WRITE], msg_to_log, to_write) < 0) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }

                    //sleep(1);

                } else if (msg_from_A[i] == 'D') {
                    sprintf(msg_to_D, "%c %c", msg_from_A[i], msg_from_A[i+2]);
                    to_write = strlen(msg_to_D);
                    if (write(fd_B_D[WRITE], msg_to_D, to_write) != to_write) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }

                    sprintf(msg_to_log, "B sent to D: %s\n", msg_to_D);
                    to_write = strlen(msg_to_log);
                    if (write(fd_log[WRITE], msg_to_log, to_write) < 0) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }
                    //sleep(5);
                }
            }
        }

        if (n_bytes < 0) {
            fprintf(stderr, "%d: read(): %s\n", __LINE__, strerror(errno));
            exit(EXIT_FAILURE);
        }

        exit(EXIT_SUCCESS);
    }

    if ((pid = fork()) < 0) {
        fprintf(stderr, "%d: fork(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (close(fd_B_C[WRITE]) < 0) {
        fprintf(stderr, "%d: close(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (close(fd_B_D[WRITE]) < 0) {
        fprintf(stderr, "%d: close(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pid == 0) {
        // Child C

        int n_bytes;
        while ((n_bytes = read(fd_B_C[READ], msg_B_C, sizeof(msg_B_C))) > 0) {
            for (i = 0; i+2 < n_bytes; i += 3) {
                if (msg_B_C[i] == 'C') {
                    sprintf(msg_C_log, "%c %c", msg_B_C[i], msg_B_C[i+2]);
                    sprintf(msg_to_log, "C sent to log: %s\n", msg_C_log);
                    size_t to_write = strlen(msg_to_log);
                    if (write(fd_log[WRITE], msg_to_log, to_write) != to_write) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }
                }
            }
        }

        if (n_bytes < 0) {
            fprintf(stderr, "%d: read(): %s\n", __LINE__, strerror(errno));
            exit(EXIT_FAILURE);
        }

        exit(EXIT_SUCCESS);
    }

    if ((pid = fork()) < 0) {
        fprintf(stderr, "%d: fork(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    if (pid == 0) {
        // Child D

        int n_bytes;
        while ((n_bytes = read(fd_B_D[READ], msg_B_D, sizeof(msg_B_D))) > 0) {
            for (i = 0; i+2 < n_bytes; i += 3) {
                if (msg_B_D[i] == 'D') {
                    sprintf(msg_D_log, "%c %c", msg_B_D[i], msg_B_D[i+2]);
                    sprintf(msg_to_log, "D sent to log: %s\n", msg_D_log);
                    size_t to_write = strlen(msg_to_log);
                    if (write(fd_log[WRITE], msg_to_log, to_write) != to_write) {
                        fprintf(stderr, "%d: write(): %s\n", __LINE__, strerror(errno));
                        exit(EXIT_FAILURE);
                    }
                }
            }
        }

        if (n_bytes < 0) {
            fprintf(stderr, "%d: read(): %s\n", __LINE__, strerror(errno));
            exit(EXIT_FAILURE);
        }

        exit(EXIT_SUCCESS);
    }

    // Parent

    if (close(fd_log[WRITE]) < 0) {
        fprintf(stderr, "%d: close(): %s\n", __LINE__, strerror(errno));
        exit(EXIT_FAILURE);
    }

    int n_bytes;
    while ((n_bytes = read(fd_log[READ], msg_from_log, sizeof(msg_from_log)-1)) > 0) {
        msg_from_log[n_bytes] = '\0';
        printf("Log pipe reads (%d) bytes:\n%s", n_bytes, msg_from_log);
        //sleep(1);
    }

    return 0;
}

There is still a lot of room for improvement. You don't need so many character array variables - remember that the memory is not shared between parent and child processes. You could just have 1 or 2 buffers to print the intermediate messages before writing them. But it seems to be working:

filipe@filipe-Kubuntu:~/dev$ ./a.out 

Written by Nawar Youssef
Log pipe reads (170) bytes:
A sent to B: C 0
A sent to B: D 1
A sent to B: C 2
A sent to B: C 3
A sent to B: C 4
A sent to B: C 5
A sent to B: D 6
A sent to B: D 7
A sent to B: C 8
A sent to B: C 9
Log pipe reads (170) bytes:
B sent to C: C 0
B sent to D: D 1
B sent to C: C 2
B sent to C: C 3
B sent to C: C 4
B sent to C: C 5
B sent to D: D 6
B sent to D: D 7
B sent to C: C 8
B sent to C: C 9
Log pipe reads (57) bytes:
D sent to log: D 1
D sent to log: D 6
D sent to log: D 7
Log pipe reads (133) bytes:
C sent to log: C 0
C sent to log: C 2
C sent to log: C 3
C sent to log: C 4
C sent to log: C 5
C sent to log: C 8
C sent to log: C 9
filipe@filipe-Kubuntu:~/dev$