C Memory assignment, Segmentation Fault / Double Free In Array of Structs

Question

From what I understand, Segmentation Fault is when you haven't assigned memory properly yet, and Double free is when you try to free memory that you already freed?

What would be the proper way to increase the size of an array of Structs, and where/which parts do you actually need to free?

I have a struct:

struct Data {
    // Some variables
}

and I'm initializing the array of those structs with:

int curEntries = 100;
int counter = 0; 
struct Data *entries = (struct Data *)malloc(curEntries * sizeof(struct Data));

When I read data from a bin file into this array and populate each of the structs, the program works up until there are more than 100 structs needed. At that time, I have the following code to realloc the array:

if (counter == curEntries - 1) { // counter = current index, curEntries = size of the array
    entries = (struct Data *)realloc(entries, curEntries * 2 * sizeof(struct Data));
    // struct Data *temp = (struct Data *)realloc(entries, curEntries * 2 * sizeof(struct Data));
    // free(entries);
    // entries = temp;
    // free(temp);
}

The line I'm using now (entries = . . . ) works, but is obviously wrong because I'm not freeing anything, right?

But when I tried using the commented out code instead, I got a double Free error

Finally, (because there are a series of automatic tests), apparently I need to use malloc and so forth in other parts of my code as well. Where else should I/do I need to assign memory?

[Please see this discussion on why not to cast the return value of malloc() and family in C..](https://stackoverflow.com/q/605845/2173917) — Sourav Ghosh, Aug 21 '17 at 12:24
[Documentation for `realloc()` is here](http://port70.net/~nsz/c/c11/n1570.html#7.22.3.5) and [here](http://man7.org/linux/man-pages/man3/malloc.3.html) and [here](http://man7.org/linux/man-pages/man3/realloc.3p.html) and [here](http://pubs.opengroup.org/onlinepubs/9699919799/functions/realloc.html) ... *sigh* — alk, Aug 21 '17 at 12:30
Segmentation fault just means "some kind of memory-related bug" and could be caused by anything. [Definitive List of Common Reasons for Segmentation Faults](https://stackoverflow.com/questions/33047452/definitive-list-of-common-reasons-for-segmentation-faults). "Double free" is not really a formal term, info here: [What does “double free” mean?](https://stackoverflow.com/questions/21057393/what-does-double-free-mean). — Lundin, Aug 21 '17 at 12:44

score 3 · Answer 1 · answered Aug 21 '17 at 12:26

3

The line I'm using now (entries = . . . ) works, but is obviously wrong because I'm not freeing anything, right?

It's wrong only if realloc() fails. When successful, realloc() automatically frees the previously allocated block if necessary (it might be not necessary if it's the same block and the system could simply change the size).

So, the common idiom looks like this:

mytype *var = malloc(...);

// ...

mytype *tmp = realloc(var, ...);
if (!tmp)
{
    free(var);
    return -1; // or whatever error
}
var = tmp;

// ...

free(var);

answered Aug 21 '17 at 12:26

So that if condition checks if the realloc failed? Could you please explain why you would free(var) at that time, wouldn't you want to keep it because there was no successful realloc? – Astantos Aug 21 '17 at 12:37
You *can* keep it, just most of the time, the block of the original size isn't helpful for your program, so you `free()` it and indicate the error. – Aug 21 '17 at 12:43

Sourav Ghosh · Answer 2 · 2017-08-21T12:45:25.967

First of all, please don't use a format like

 pointerVar = realloc (pointerVar , newsize);  // use the same pointer variable

because, in case realloc() fails, you'll wipe the actual pointer, too.

For the case when realloc() fails, from C11, chapter §7.22.3.5,

The realloc function returns ... a null pointer if the new object could not be allocated.

and

[....] If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged.

The proper way of using realloc will be

  tempPtr = realloc (oldPtr, newSize);

  if ( tempPtr )  //allocation successful, oldPtr is `free()`-d can be reused now
  {
      oldPtr = tempPtr;
  } // and continue using `oldPtr`
  else
  {
      // some error handling
      // can still make use of `oldPtr`
   }

That said, realloc() takes care of cleaning up the previous memory allocation on it's own in case the new memory is allocated successfully, you don't need to free it.

Quoting C11, same chapter

void *realloc(void *ptr, size_t size);
The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size.

So, in case of your commented out code

struct Data *temp = (struct Data *) realloc(entries, curEntries * 2 * sizeof(struct Data));
  //assume success, memory pointed to by entries will be automatically freed

free(entries);
   // now, you're trying to free already freed memory, UB....

entries = temp;
free(temp);

You said that if realloc fails, I'd wipe the pointer, but in C11 I don't need to worry because "the old object is not deallocated", is that right? — Astantos, Aug 21 '17 at 12:38

chqrlie · Accepted Answer · 2017-08-21T13:21:09.010

You get a double free error because your call to realloc() succeeds so the previous pointer has been freed and yet you call free(entries). The library can sometimes determine that a block has already been freed but this sanity check is not always effective. The C Standard does not provide any guarantee to this regard, passing a freed pointer to free() has undefined behavior.

On a system with memory protection, a segmentation fault may occur when you try and read or write to a memory address that has not been assigned to your process, or that has been invalidated for the process. Dereferencing a pointer to a freed block may cause a segmentation fault before the library can determine that the block has already been freed.

The scheme for your reallocating your array should be this:

size_t curEntries = 100; // size of the array
size_t counter = 0;      // current index

...

if (counter == curEntries) {
    // array is full, try and reallocate to a larger size
    size_t newSize = curEntries * 2;
    struct Data *newArray = realloc(entries, newSize * sizeof(*newArray));
    if (newArray == NULL) {
        // cannot reallocate, out of memory.
        // handle this error, entries is still valid.
        abort();
    } else {
        // array was reallocated possibly to a different address
        // entries is no longer a valid pointer
        entries = newArray;     // update array pointer
        curEntries = newSize;   // update number of entries
    }
}

sir why is malloc sizeof(*newArray) instead of sizeof(newArray) ? — Soner from The Ottoman Empire, Aug 21 '17 at 13:40
@snr: `T *p = malloc(count * sizeof(*p));` is a safer way to allocate an array of `count` elements of type `T`. Using `sizeof(*p)`, the size of the object pointed to by `p`, is correct and does not depend on the type of `p`. It avoids potential inconsistency between the definition of `p` and the type used in the allocation size. `sizeof(newArray)` would be the size of the pointer, not the size of the structure it points to. — chqrlie, Aug 22 '17 at 06:11

C Memory assignment, Segmentation Fault / Double Free In Array of Structs

3 Answers3