Calloc & realloc: Error in `./a.out': free(): invalid next size (normal)

Question

I have a little piece of code (in C) where I'am allocating an array and scaning numbers in it. If the array is too small I'm reallocating memory for my array. Sometimes it works fine but sometimes it returns "Error in `./a.out': free(): invalid next size (normal)."

Here is the code:

int *array;
int memory=0, i=0, scanreturn=0;
array = (int *)calloc(30, sizeof(int));
/*Allocating 30 ints*/
while ( scanf("%d", &array[i]) != EOF){
  if(i == (memory - 5)){
    /*There's only 5 ints left. Allocating 10 more*/
    memory = memory + 10;
    array = (realloc(array, memory * sizeof *array));
  }
  i++;
}
...
free(array);

I suppose it's cause freeing the memory at the end but I really don't know how to figure it out.

That's not an [MCVE](https://stackoverflow.com/help/mcve). Also, [Don't cast the result of malloc (and friends)](http://stackoverflow.com/q/605845). (You don't even consistently violate that guideline.) — Deduplicator, Nov 12 '14 at 21:02
In addition to @Deduplicator's comment, see [c-faq 7.7b What's wrong with casting `malloc`'s return value?](http://c-faq.com/malloc/mallocnocast.html) — , Nov 12 '14 at 21:06
Compile with all warnings & debug info (`gcc -Wall -Wextra -g`). Then **use the debugger** (`gdb`) and use [valgrind](http://valgrind.org/) — Basile Starynkevitch, Nov 12 '14 at 21:10
regarding this line: array = (int *)calloc(30, sizeof(int)); the code should always check the contents of array to assure that the calloc was successful I.E. contents not NULL — user3629249, Nov 13 '14 at 06:05
regarding this line: array = (realloc(array, memory * sizeof *array)); 1) there is an extra set of parens that should be removed. 2) the original variable pointer 'array' should NOT be used as the receiving variable, because if the realloc function fails, then the original pointer is lost. — user3629249, Nov 13 '14 at 06:07
this line: while ( scanf("%d", &array[i]) != EOF){ 1) the scanf, after the first, will probably fail as the intervening newline has not been consumed suggest the format string be " %d", notice leading space. 2) the returned value from scanf needs to be checked to assure that the input was successful — user3629249, Nov 13 '14 at 06:09
regarding this line: if(i == (memory - 5)){ i starts at 0 and increments and memory starts at 0 and is not modified unless this if evaluates to true, so this IF code block will never be entered. — user3629249, Nov 13 '14 at 06:16
if memory were initialized to 30 and used in the original calloc() then the code has some chance of working. However, why alloc 10 more int sized entries when there are still 5 not yet used, Better to check if i+1 == memory then memory+= 10; — user3629249, Nov 13 '14 at 06:18
this line: array = (realloc(array, memory * sizeof *array)); 1) is using 'memory' which at the initial pass through this code block is set to 10 (0+10) which is far smaller than the initial 30 allocation. 2) sizeof is a compile time operator, so knows nothing about the current size of array. — user3629249, Nov 13 '14 at 06:21

KevinDTimm · Answer 1 · 2014-11-12T21:14:18.197

Questions to ask yourself:

1) Since you have no error checking, it's hard to say whether any of your alloc's are working
2) i and memory start as 0, when will realloc ever be called?
2a) if realloc isn't called, what happens when you get to the end of your original alloc?

P.S. Never cast the result of an alloc
P.P.S.
values of i 0, 1, 2, 3, ....
values of memory 0, 0, 0, 0, 0, .....
values of memory - 5 -5, -5, -5, -5, .....
if (i == (memory - 5))
0 == -5 FALSE
1 == -5 FALSE
2 == -5 FALSE
3 == -5 FALSE
......

As you can see, it will take a long time for this to not be FALSE

score 1 · Answer 2 · answered Nov 12 '14 at 21:09

With your starting conditions for i and memory, you never enter the if statement in the loop
if(i == (memory - 5)), array is never reallocated, and when you scanf() into the array you are writing out of bounds. This is undefined behaviour and aparently you overwrite some internal data, because you get a warning when you call free().

score 1 · Answer 3 · answered Nov 12 '14 at 21:09

int *array;
int memory=0, i=0, scanreturn=0;
array = (int *)calloc(30, sizeof(int));
/*Allocating 30 ints*/
while ( scanf("%d", &array[i]) != EOF){
  if(i == (memory - 5)){

What have we here? How can this if ever trigger? memory is set to 0 and never modified and i starts at 0 and gets incremented with every iteration of the loop. So your loop compares:

 i == (memory - 5)  result
 0 == (0 - 5)        false
 1 == (0 - 5)        false
 2 == (0 - 5)        false
           ...
30 == (0 - 5)        false
31 == (0 - 5)        false

As a result, you never get into the code which attempts to grow the buffer and you end up with a buffer overflow. After that, you're in undefined behavior land and anything goes.

The obvious solution? Set memory = 30 instead of memory = 0.

Also note that you never check the result of calloc or realloc. While it's extremely unlikely for a memory operation to fail with modern operating systems, you should not just ignore the possibility of an error.

R Sahu · Accepted Answer · 2014-11-12T21:19:41.967

Looking at your posted code,

int *array;
int memory=0, i=0, scanreturn=0;
array = (int *)calloc(30, sizeof(int));
/*Allocating 30 ints*/
while ( scanf("%d", &array[i]) != EOF){

  // ***********************************************
  // This conditional will never be true since
  // memory has been initialized to 0.
  // ***********************************************
  if(i == (memory - 5)){
    /*There's only 5 ints left. Allocating 10 more*/
    memory = memory + 10;
    array = (realloc(array, memory * sizeof *array));
  }

  // ***********************************************
  // What happens when 1 == 30?
  // You start writing over out of bounds memory, leading to undefined behavior.
  // ***********************************************
  i++;
}
...
free(array);

If you initialize memory correctly, the purpose of using

  if(i == (memory - 5)){

would've been served. Change the line

int memory=0, i=0, scanreturn=0;

to

int memory=30, i=0, scanreturn=0;

to solve the problem.

It's perfectly clear what he wants to accomplish: to grow `array` as soon as he's close to running out of space. He just does it wrong. — Nik Bougalis, Nov 12 '14 at 21:13

score 0 · Answer 5 · answered Nov 12 '14 at 21:15

Cleaned up version of your code:

int memory=0, i=0;
array = calloc(30, sizeof(int));

while (scanf("%d", &array[i]) != EOF) {
  if (i == (memory - 5)) {
    memory = memory + 10;
    array = realloc(array, memory * sizeof *array);
  }
  i++;
}

You need to initialize memory = 30. With memory = 0, your comparison will become if (i == -5)..., and realloc() will never be called.

You should also check the return value of calloc() and realloc() in case the allocations fail.

Calloc & realloc: Error in `./a.out': free(): invalid next size (normal)

5 Answers5