1

This question is definitely related to this and it's answer is what I based my function off of.

char *get_next_line(FILE *fp) {
char ch = 0;
int CUR_MAX = 4095;
char *buffer = (char*) malloc(sizeof(char) * CUR_MAX); // allocate buffer.
char *temp = (char*) malloc(sizeof(char) * CUR_MAX); // allocate buffer.
int count = 0;
int length = 0;

while ((ch != '\n')) {
  if (ch == '\377') { return NULL; }
  if(count ==CUR_MAX) {
    CUR_MAX *= 2;
    count = 0;
    if ((temp = realloc(buffer, CUR_MAX)) != NULL) {
      buffer = temp;
      free(temp);
    }
  }
  ch = getc(fp);
  buffer[length] = ch;
  length++;
  count++;
}

For some reason, when reading in very large strings I'm faced with:
glibc detected - realloc() invalid next size.

Is there something I'm missing here?

Thanks!

Community
  • 1
  • 1
user652650
  • 648
  • 6
  • 18
  • 5
    Not directly related, but you should never store the result of `realloc` in the original variable. If the `realloc` fails and returns `NULL`, the original block of memory will be orphaned, creating a memory leak. Instead, store `realloc`'s return value in a temporary variable, check it for `NULL`, and copy it to the original variable only if it's valid. – Adam Liss Nov 12 '12 at 01:17
  • Is it possible CUR_MAX is overflowing (becoming negative)? On a side not, the result of realloc should be assign to a temp variable in case it returns null (with your approach you would lose your reference to the buffer in this situation). – jpm Nov 12 '12 at 01:18
  • Edited to store result of realloc in a temp var, then set to buffer, then freed temp. The issue goes away but a seg fault still occurs. – user652650 Nov 12 '12 at 01:20
  • 1
    This has nothing to do with your question but _please_ clean up your code of extraneous whitespace, you have a ridiculous amount of trailing whitespace which makes viewing your code in a mobile browser unnecessarily difficult. – Jeff Mercado Nov 12 '12 at 01:26

3 Answers3

4

When you:

free(temp);

buffer is also freed, since:

buffer = temp;

So later on you're trying to:

realloc(buffer, CUR_MAX)

which cannot work because buffer has been freed indirectly through temp. You cannot realloc() a freed pointer.

Nikos C.
  • 50,738
  • 9
  • 71
  • 96
2

I believe you're accessing memory outside of your malloced/realloced region.

Not setting count = 0 inside your while look should fix this.


To try and elaborate...

When starting the formula:

count = 0
length = 0
CUR_MAX = 4095

These will increment until they reach 4095. Once we reach 4095, we will have:

count = 0
length = 4096
CUR_MAX = 8190

We will then increment until count is 8190. Shorly before then, we have:

count = 8100
length = 12196
CUR_MAX = 8190

Your array is only 8190 in length, but you're dereferencing buffer[12196], which is an invalid index.


Also, you probably want to handle the else case for when temp == NULL. This probably can be handled by an assert or a big old failure. And don't free(temp) inside your success side. That's freeing the memory you just attempted to allocate. And is causing your new segmentation fault.

Bill Lynch
  • 80,138
  • 16
  • 128
  • 173
-1

realloc(buffer, CUR_MAX)) returns pointer to void.

I believe the correct statement is (char *)realloc(buffer, CUR_MAX))

Rad'Val
  • 8,895
  • 9
  • 62
  • 92
  • Nope, the cast is not necessary in C (unlike C++) due to implicit conversion of `void*`. See [here](http://stackoverflow.com/questions/3559656/casting-void-pointers) for discussion. – Mac Dec 02 '12 at 23:06