C: Error using malloc and realloc on array of structs

Question

I am trying to dynamically allocate memory for a algorithm on a graph. This is my code broken down to the problem:

typedef struct
{
    int id;
    int color;
} node_t;

unsigned int max_n = 2;
node_t* node_malloc = malloc(sizeof(node_t) * max_n);
node_t* nodes = node_malloc;

if(atoi(left_node) > max_n)
{
    max_n = (size_t) atoi(left_node);
    nodes = realloc(nodes, sizeof(node_t) * max_n);
}

For max_n < 9 the realloc works - when max_n is 9 or greater, "free(): invalid pointer:" is thrown.

free(node_malloc);

When free(node_malloc) is called after a realloc happened it throws "free(): invalid next size (fast)"

Thank you for your answers!

Yea sorry, added that. I does work when it didnt reallocate, but not when it reallocated. — awfullynice
– awfullynice, Commented Nov 20, 2020 at 16:46
it also depends on how you write to array. see nothing good in atoi() and all this. and put to account that you dont care to pack struct's so there're many gaps and it will definitely turn out bad when something custom is done on that. — Алексей Неудачин
– Алексей Неудачин, Commented Nov 20, 2020 at 17:21

John Bode · Accepted Answer · 2020-11-20 16:54:31Z

2

What's happened is that after the realloc call, the pointer value in nodes_malloc is no longer valid. If realloc cannot extend the current buffer in place, it will allocate space for a new buffer, copy the contents of the old buffer to it, and then free the old buffer, which is obviously what happens when you extend by more than 9 elements.

Use nodes as a temporary target of the realloc call - if it's not NULL (meaning the realloc succeeded), assign it back to nodes_malloc:

nodes = realloc( nodes_malloc, sizeof *nodes * max_n );
if ( nodes )
{
  nodes_malloc = nodes;
}
else
{
  // realloc failed
}

answered Nov 20, 2020 at 16:54

John Bode

125k19 gold badges130 silver badges211 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Wör Du Schnaffzig Over a year ago

Yes and put it into an own function. You probably need to use it more than once. ;)

awfullynice Over a year ago

Thanks. I did as you said, and free(node_malloc) works fine now. Now tho: When i call realloc(nodes_malloc, sizeof *nodes * 29); i get "free(): invalid size" !

Some programmer dude · Accepted Answer · 2020-11-20 16:50:50Z

You have this:

node_t* node_malloc = malloc(sizeof(node_t) * max_n);
node_t* nodes = node_malloc;

After that, the pointers node_malloc and nodes both point to the very same memory.

But after the first call to realloc that's no longer guaranteed. In fact it's almost certain that nodes will point somewhere else, and the node_malloc pointer will be invalid.

Attempting to free(node_malloc) will then lead to undefined behavior.

On a related issue, you should never assign back to the pointer you pass to realloc. If realloc fails it will return NULL, but the old memory allocation will remain valid. If you reassign back to the same pointer, you loose the original memory and have a memory leak. Always use a temporary variable, and check for failure (NULL).

OP should probably replace free(node_malloc) with free(nodes).

Collectives™ on Stack Overflow

C: Error using malloc and realloc on array of structs

2 Answers 2

2 Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Related