Reputation: 43
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
Upvotes: 0
Views: 2375
Reputation: 409196
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.
Upvotes: 3
Reputation: 6121
realloc expects the buffer which need to reallocated as its first parameter.
realloc(*buffer, buffSiz);
Upvotes: 1
Reputation: 182649
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.
Upvotes: 4