0

What should be incredible simple has me stumped. All I am doing is writing a getLine method that should read a line into a buffer. I allocate memory to the buffer starting at 10 bytes, then fgets into a char array. The idea is that I attempt this over and over, doubling the size of the buffer, until it gets the whole line. However what I seem to be observing is that it reads a portion of the line, and then on the next attempt continues where it ran out of room on the last attempt, so it gives the last part of the line. Any insight it to what is wrong, I expect with my realloc memory to the buffer, would be much appreciated.

char * mygetline( char **buffer,  FILE * infile )
{
    int buffSiz = 10;
    *buffer = calloc( buffSiz, 1 );
    do
    {
        char * result = fgets(*buffer, sizeof *buffer ,infile);
        if(result == NULL)
        {
            free(buffer);
            return NULL;
        }

        if(strchr(*buffer, '\n'))
            return *buffer;
        else
        {
            char *newBuf;
            buffSiz = buffSiz*2;
            newBuf = realloc(NULL, buffSiz);
            char *buffer = newBuf;
            printf("%d", buffSiz);
            printf("%s", *buffer);
        }
    } while (1);  // INFINITE LOOP - WE ONLY GET OUT BY RETURNING FROM WITHIN
Mat
  • 202,337
  • 40
  • 393
  • 406
user1781671
  • 43
  • 1
  • 1
  • 5
  • 1
    I think the idea is, you `realloc` the old buffer (old data gets copied automatically), then continue reading to the (now bigger) buffer. – hyde Feb 01 '13 at 07:54

3 Answers3

4

There are multiple flaws:

  • sizeof *buffer doesn't do what you want since it's a pointer. You should use buffSiz instead

  • char *buffer = newBuf; you're assigning the new memory to a local variable, i.e. a variable that disappears when the else is exited

  • you're not passing your buffer to realloc, you're always passing NULL

  • you're not checking the return of realloc

As a shameless self-promotion, here's a function I posted in another answer.

Community
  • 1
  • 1
cnicutar
  • 178,505
  • 25
  • 365
  • 392
3

That's how the file functions works. You have to get the file position at the start, then reset the position every time you attempt a new read.

You can get the current position with the ftell function, and then set the position with the fseek function.

Instead of doing that (getting position and constant repositioning the file position), why not use the fact that realloc reallocates the passed in buffer and keeps the contents together with the fact that the read functions reads the next part? In that case you need to have a pointer to where to read next, and if no newline then just reallocate and advance the pointer?

Something like this:

char *readpos = *buffer;
size_t readsize = bufSize;

while (1)
{
    char *p = fgets(readpos, readsize, infile);
    if (!p)
        break;  /* Error */

    /* Search from the end, as there's where the newline should be */
    if (strrchr(p, '\n') != NULL)
        return *buffer;  /* Found the newline */

    /* Need to allocate more memory */
    bufSize += bufsize;
    *buffer = realloc(*buffer, bufSize);

    /* Set the pointer to next position to read into */
    readpos = *buffer + strlen(buffer);

    /* Loop takes case of trying again */
}

Note: The above code was written in the browser, not tested, and might need some tuning to make it work properly.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

realloc expects the buffer which need to reallocated as its first parameter. 

realloc(*buffer, buffSiz);
sr01853
  • 6,043
  • 1
  • 19
  • 39