read(newsockfd, buffer ,to read) sometimes getting struck

gajananh999 · 09-17-2012, 01:44 AM

Hello Everyone,

I have written socket programming in c++.
some times my read will get strucked.

Code:


void handlesocket(int newsockfd)
{
        char *buffer, *temp, *temp2;
        int n,i;
        int readNow, toRead;
        if (newsockfd < 0)
                error("ERROR on accept");

        buffer = new char[960000];
        temp = new char[16];
        temp[0] = '\0';
        temp2 = new char[1];
        temp2[0] = '\0';
        i = 0;
        do
        {
                n = read(newsockfd,temp2, 1);
                if(temp2[0]=='@') break;
                temp[i++] = temp2[0];
                temp[i] = '\0';
        }
        while(n > 0);

        n = 0;
        do
        {
                buffer[0] = '\0';
                toRead = atoi(temp) - n;
          printf("ready to read\n ToRead=%d\n",toRead);
          //Sometime strucking at this next line
             readNow = read(newsockfd,buffer,toRead);
                printf("ReadNow is %d\n",readNow);
                if(readNow < 0) break;
                n += readNow;
                printf("ISAC %d --- %d --- %d\n",atoi(temp),n,readNow);
                buffer[readNow] = '\0';
                writeintofile(buffer);
        }
        while(n < atoi(temp));
  close(newsockfd);
        free(buffer);
        free(temp);
        free(temp2);
}

Please help me out...

ninis666 · 09-17-2012, 10:51 AM

1/ You dont check if the lenght is correct (you're expecting to read more than what you'll have to).
2/ You have to test the return value of read system call (take a look into the manpage).

By the way, dont you have to allocate memory for your temporary variables (except for your read buffer, it can't fit in stack in some systems). I think that you should use something like this :

Code:

void handlesocket(int newsockfd)
{
#define MAX 960000
    int i, n, lenght, done;
    char temp2;
    char * buffer;
    char temp[16];

    if (newsockfd < 0) {
	fprintf(stderr, "Failed to read lenght from socket : %m\n");
	return;
    }

    i = 0;
    while (1) {
	n = read(newsockfd, &temp2, 1);
	if (n < 0) {
	    if (errno != EAGAIN && errno != EWOULDBLOCK) {
		fprintf(stderr, "Failed to read lenght from socket : %m\n");
		return;
	    }

	    n = 0;
	}

	if (temp2 == '@')
	    break;

        if (i >= sizeof temp) {
            fprintf(stderr, "Toooo long lenght received (max = %d, %d received so far)\n", sizeof temp, i);
            return;
        }

	temp[i++] = temp2;
	temp[i] = '\0';
    }

    lenght = atoi(temp);
    if (lenght <= 0 || lenght >= MAX) {
	fprintf(stderr, "Invalid lenght received : %s\n", temp);
	return;
    }

    buffer = new char [lenght + 1];
    if (buffer == NULL) {
	fprintf(stderr, "Cannot create buffer of %db\n", lenght + 1);
	return;
    }

    printf("Reading %d bytes\n", lenght);

    do {

	n = read(newsockfd, buffer + done, lenght - done);
	if (n < 0) {
	    if (errno != EAGAIN && errno != EWOULDBLOCK) {
		fprintf(stderr, "Failed to read data from socket : %m\n");
		return;
	    }
	    n = 0;
	}

	done += n;

    } while (done < lenght);

    buffer[lenght] = 0;
    writeintofile(buffer);
    close(newsockfd);
    delete buffer;
}