CODEWITHSUNDEEP
clinuxgcc
Soham
Soham

Reputation: 873

Program using read() entering into an infinite loop

1oid ReadBinary(char *infile,HXmap* AssetMap)
{
    int fd; 
   size_t bytes_read, bytes_expected = 100000000*sizeof(char); 
   char *data;

   if ((fd = open(infile,O_RDONLY)) < 0) 
      err(EX_NOINPUT, "%s", infile);


   if ((data = malloc(bytes_expected)) == NULL)
      err(EX_OSERR, "data malloc");

   bytes_read = read(fd, data, bytes_expected);

   if (bytes_read != bytes_expected) 
      printf("Read only %d of %d bytes %d\n", \
         bytes_read, bytes_expected,EX_DATAERR);

   /* ... operate on data ... */
    printf("\n");
    int i=0;
    int counter=0;
    char ch=data[0];
    char message[512];
    Message* newMessage;
    while(i!=bytes_read)
    {

        while(ch!='\n')
        {
        message[counter]=ch;
        i++;
        counter++;
        ch =data[i];
        }
    message[counter]='\n';
    message[counter+1]='\0';
//---------------------------------------------------
    newMessage = (Message*)parser(message);
    MessageProcess(newMessage,AssetMap);
//--------------------------------------------------    
    //printf("idNUM %e\n",newMessage->idNum);
    free(newMessage);
    i++;
    counter=0;
    ch =data[i];
    }
   free(data);  

}

Here, I have allocated 100MB of data with malloc, and passed a file big enough(not 500MB) size of 926KB about. When I pass small files, it reads and exits like a charm, but when I pass a big enough file, the program executes till some point after which it just hangs. I suspect it either entered an infinite loop, or there is memory leak.

EDIT For better understanding I stripped away all unnecessary function calls, and checked what happens, when given a large file as input. I have attached the modified code

void ReadBinary(char *infile,HXmap* AssetMap)
{
    int fd; 
   size_t bytes_read, bytes_expected = 500000000*sizeof(char); 
   char *data;

   if ((fd = open(infile,O_RDONLY)) < 0) 
      err(EX_NOINPUT, "%s", infile);


   if ((data = malloc(bytes_expected)) == NULL)
      err(EX_OSERR, "data malloc");

   bytes_read = read(fd, data, bytes_expected);

   if (bytes_read != bytes_expected) 
      printf("Read only %d of %d bytes %d\n", \
         bytes_read, bytes_expected,EX_DATAERR);

   /* ... operate on data ... */
    printf("\n");
    int i=0;
    int counter=0;
    char ch=data[0];
    char message[512];
    while(i<=bytes_read)
    {

        while(ch!='\n')
        {
        message[counter]=ch;
        i++;
        counter++;
        ch =data[i];
        }
    message[counter]='\n';
    message[counter+1]='\0';
    i++;
    printf("idNUM \n");
    counter=0;
    ch =data[i];
    }
   free(data);  

}

What looks like is, it prints a whole lot of idNUM's and then poof segmentation fault

I think this is an interesting behaviour, and to me it looks like there is some problem with memory

FURTHER EDIT I changed back the i!=bytes_read it gives no segmentation fault. When I check for i<=bytes_read it blows past the limits in the innerloop.(courtesy gdb)

Upvotes: 1

Views: 1784

Answers (3)

Karl Bielefeldt
Karl Bielefeldt

Reputation: 49008

Try the following loop. Basically, it refactors your implementation so there is only one place where i is incremented. Having two places is what's causing your trouble.

#include <stdio.h>
#include <string.h>

int main()
{
    const char* data = "First line\nSecond line\nThird line";
    unsigned int bytes_read = strlen(data);

    unsigned int i = 0;
    unsigned int counter = 0;
    char message[512];

    while (i < bytes_read)
    {
        message[counter] = data[i];
        ++counter;
        if (data[i] == '\n')
        {
            message[counter] = '\0';
            printf("%s", message);
            counter = 0;
        }
        ++i;
    }

    // If data didn't end with a newline
    if (counter)
    {
        message[counter] = '\0';
        printf("%s\n", message);
    }

    return 0;
}

Or, you could take the "don't reinvent the wheel" approach and use a standard strtok call:

#include <stdio.h>
#include <string.h>

int main()
{
    char data[] = "First line\nSecond line\nThird line";
    char* message = strtok(data, "\n");

    while (message)
    {
        printf("%s\n", message);
        message = strtok(NULL, "\n");
    }

        return 0;
}

Upvotes: 1

charleyc
charleyc

Reputation: 1709

Is it possible that on the system you're using, 500,000,000 is larger than the largest size_t? If so, bytes_expected may be rolling over to some smaller value. Then bytes_read is following suit, and you're ending up taking a smaller chunk of data than you actually expect. The result would be that for large data, the last character of data is unlikely to be a '\n', so you blow right past it in that inner loop and start accessing characters beyond the end of data. Segfault follows.

Upvotes: 0

NPE
NPE

Reputation: 500167

The most glaring problem is this:

    while(ch!='\n')
    {
    message[counter]=ch;
    i++;
    counter++;
    ch =data[i];
    }

Unless the last character of the file (or the block that you've just read) is \n, you'll go past the end of the data array, most probably smashing the stack along the way (since you're not checking whether your write to message is within bounds).

Upvotes: 3

Related Questions