tags:

views:

28

answers:

1

Hi

im trying to read data from pipe using poll, here is code:

#include <stdio.h>
#include <fcntl.h>
#include <poll.h>
#define MAX_BUFF_SIZE 128

int main(void){
    char buff[MAX_BUFF_SIZE];
    char *buf;
    int fd,ret,len;
    struct pollfd fdinfo[1];

    if((fd = open("/home/s/pipe", O_RDONLY)) == NULL) {
        printf("Error Opening File.\n");
        exit(1);
    }

    fdinfo[0].fd = fd;
    fdinfo[0].events = POLLIN|POLLPRI ;

    while(1){
        ret = poll(fdinfo,1,-1);
        if (ret < 0) printf("\n\r error");

        if ( ((fdinfo[0].revents&POLLIN) == POLLIN) || ((fdinfo[0].revents&POLLPRI) == POLLPRI) ){
            len = read(fdinfo[0].fd, buff, MAX_BUFF_SIZE);
            if (len > 0){
                buff[len] = 0;
                buf = (char *) malloc(len);
                memcpy(buf, buff, len);
                printf("\n read len %d\n %s",len, buf);
                free(buf);
             }
        }

    }
    return 0;
}

It does not seems to work fine - after each properly data i got a lot of garbage in output, but i dont know where it comes frome. So im asking for help.

thx in advance

+1  A: 

If there is at least MAX_BUFF_SIZE bytes available, len will be set to MAX_BUFF_SIZE, and buff[len] will write past the end of buff. After that, all bets are off. You can fix this by passing MAX_BUFF_SIZE - 1 to read().

However, more worryingly, you're not copying the \0 in your memcpy() call, as the number of bytes including the \0 is len + 1.

While we're at it:

  • don't cast the return from malloc()
  • check the return from malloc() isn't NULL
  • if poll() errors, you're still checking the revents field, which won't be applicable any more
  • your test if ( ((fdinfo[0].revents&POLLIN) == POLLIN) || ((fdinfo[0].revents&POLLPRI) == POLLPRI) ) could better be written if (fdinfo[0].revents & (POLLIN | POLLPRI)) but shouldn't be necessary anyway, as you've asked poll() to look for these two things only
Tim
+1 on all the corrections, except maybe the casting malloc() one. With modern standards, this shouldn't be a problem, and I see it as a good reminder of what kind of pointers you're dealing with.
Santiago Lezica