Can't read more than one word from named pipe, using poll

Question

I am simulating having two writers and one reader, based on this answer.

So, I create two pipes and I write one actual string to every pipe and one string which notifies the reader that he is done with this writer.

However, it will only read the first string and sometimes the ending string of the 2nd pipe.

What am I missing?

reader.c

int main() {
    int w_no = 2;
    int fd[w_no];
    char * myfifo[w_no];
    fill_names(myfifo, w_no);
    print_names(myfifo, w_no);
    struct pollfd fdtab[w_no];
    int done[w_no];

    /* create the FIFO (named pipe) */
    int i;
    for (i = 0; i < w_no; ++i) {
        //fd[i] = open(myfifo[i], O_RDONLY);
        while( (fd[i] = open(myfifo[i], O_RDONLY)) == -1); 

        fdtab[i].fd = fd[i];
        fdtab[i].events = POLLIN;
        fdtab[i].revents = 0;

        done[i] = 0;
    }

    char buffer[1024];
    ssize_t bytes;
    printf("Edw prin\n");
    while(not_all_done(done, w_no)) {
        int retpoll = poll(fdtab,  w_no, 300);
        if(retpoll != 0) {
             if (retpoll == -1) {
                 perror("poll");
                 break;
             }

             for(i = 0; i < w_no; ++i) {
                 if(fdtab[i].revents & POLLIN) {
                            printf("Edw %d %d %d %d\n", i, retpoll, fdtab[i].revents, POLLIN);
                     //read the written pipe
                     while((bytes = read(fdtab[i].fd, buffer, sizeof(buffer))) > 0)
                        printf("Read |%s| %d %d %d\n", buffer, retpoll, fdtab[i].revents, POLLIN);
                     if(!strcmp(buffer, "++"))
                        done[i] = 1;
                 }
             }
         } else if (retpoll == 0) {
                /* the poll has timed out, nothing can be read or written */
                printf("timeout from writer\n");
                break;
            }
    }

    for (i = 0; i < w_no; ++i) {
        close(fd[i]);
    }

    free_names(myfifo, w_no);
    return 0;
}

writer.c

int main() {
    int w_no = 2;
    int fd[w_no];
    char * myfifo[w_no];
    fill_names(myfifo, w_no);
    print_names(myfifo, w_no);

    /* create the FIFO (named pipe) */
    int i;
    int bytes;
    for (i = 0; i < w_no; ++i) {
      mkfifo(myfifo[i], 0666);
        fd[i] = open(myfifo[i], O_WRONLY);

        while( (bytes = write(fd[i], "Hi+", sizeof("Hi+"))) == 3);
        printf("wrote %d bytes, %d\n", bytes, sizeof("Hi+"));
        while( (bytes = write(fd[i], "++", sizeof("++"))) == 2);
        printf("wrote %d bytes, %d\n", bytes, sizeof("++"));
    }

    for (i = 0; i < w_no; ++i) {
        close(fd[i]);
        unlink(myfifo[i]);
    }

    free_names(myfifo, w_no);
  return 0;
}

Sample output:

/tmp/myfifo_0
/tmp/myfifo_0
/tmp/myfifo_1
/tmp/myfifo_1
wrote 4 bytes, 4
wrote 3 bytes, 3
wrote 4 bytes, 4
Edw prin
wrote 3 bytes, 3
Edw 0 2 17 1
Read |Hi+| 2 17 1
Edw 1 2 1 1
Read |Hi+| 2 1 1
^C

---

EDIT

When the Hi+ strings are arriving the value of bytes is 7.

The ending string I am trying to send is ++, but it doesn't get read.

---

EDIT_2

char* concat(char *s1, char *s2) {
  char *result = malloc(strlen(s1) + strlen(s2) + 1);  //+1 for the null-terminator
  //in real code you would check for errors in malloc here
  strcpy(result, s1);
  strcat(result, s2);
  return result;
}

void fill_names(char* f[], int n) {
  int i = 0;
  char * buf = "/tmp/myfifo_";
  char str[15];
  for (; i < n; ++i) {
    sprintf(str, "%d", i);
    f[i] = concat(buf, str);
  }
}

---

Idea

Maybe the writer closes and unlinks the pipes before the data is read from them? If so, what should I do to prevent that?

If put a sleep(10) before that, it wont' change behaviour, it will just read the two first strings, but it will take more time and then it will hang up (because it waits the ending strings).

---

EDIT_3

I also have a main.c which execs reader and writer.

Answer 1

There is a problem in your string writing:

    while( (bytes = write(fd[i], "Hi+", sizeof("Hi+"))) == 3);
    printf("wrote %d bytes, %d\n", bytes, sizeof("Hi+"));
    while( (bytes = write(fd[i], "++", sizeof("++"))) == 2);
    printf("wrote %d bytes, %d\n", bytes, sizeof("++"));

Here you send 7 bytes: H i + \0 + + \0 , because the sizeof() of a string litteral includes the null terminator.

By the way, while((bytes=write(...))==3) will loop as long as 3 bytes could be written. This doesn't happen here because of the null terminator your writing as well. But better remove the enclosing while.

As the pipe is a stream, nothing garantees you will receive the bytes in two distinct reads. In fact, all your explanations and logs show that your receive all the 7 bytes at once.

However, you print the content using printf("Read |%s| %d %d %d\n"...): the result of prining a string that includes a '\0" is undefined. In your case, the printed string is truncated. So only "Hi+" is printed but "\0++" remains hidden in the buffer.

By the way, while((bytes = read(...)) > 0) could loop and print several times. This is not per se a problem. It's just that if the writer sends the data in time, the continuous reading could temporary lock out the reading of the other pipes. Generally in a poling programme, one prefer to read a little bit from every ready pipe.

Your check for the ending string

    if(!strcmp(buffer, "++"))
          done[i] = 1;

might in most cases not succeed. You are not sure that one write on one side will result in on read in the other side. So your "++" string will not necessarily be at the begin of the buffer. It could be anywhere in the buffer, so you have to search for it. It could even be split between two successive reads.

By the way, it might be that read() only finds back partial data (ex: "i+") without the terminating null. If you'd then try to print your buffer assuming that there is a valid string in it, you'd risk a buffer overflow.

Recommendation:

If your named pipes are meant for processing text data, I would suggest to add a '\n' at the end of each group of data you want to send, and write the strings to the pipe without the terminating null:

bytes = write(fd[i], "Hi+\n", sizeof("Hi+\n")-1);

Then, when you read, you could manage the buffer like a string: always add the trailing 0:

bytes = read(fdtab[i].fd, buffer, sizeof(buffer)-1);  // leave a byte for terminator
if (bytes>0) {
    buffer[bytes]=0; // end of string.  
    // process the string in the buffer
}
else if (bytes==0) {
    done[i]=1; 
} 

Finally for identifying your end command, assuming that you've send it as "++\n" there are three possibilities:

if (strncmp(buffer,"++\n",3)==0  /* it's at the beginning of the buffer */ 
      || strstr(buffer, "\n++\n") )    /* it's in the middle but not a subpart and preceded by a packet separator */
       done[i]=1;   

But you also have to check for splits between two reads. This is more delicate, but I'm sure you find a way ;-)

Answer 2

Try to also print the number of bytes that you read in your reader program. My guess is that you have read some more bytes than you think, but when printing the string you only get the bytes before the first terminating zero.

Was it your intention to send terminating zeros on the named pipe? Maybe it would have been better to send something else like a newline?