kadina
kadina

Reputation: 5372

Is it good coding practice to assign the address returned by realloc() to the same pointer?

I saw some code related to realloc() on some sites as below.

int *p = (int *)malloc(sizeof(int) * 10);
p = (int *)realloc(p, 100);

But as the standard says, if realloc fails, the original block is left untouched and it returns NULL.

So if realloc fails, from above example, we will lose the ability to free p. Can any one please let me know is it good coding practice to assign the address returned by realloc() to the same pointer?

Upvotes: 3

Views: 635

Answers (2)

chqrlie
chqrlie

Reputation: 145277

R. gave a clear answer to your question. Let me emphasize two other issues in the code fragment:

int *p = (int *)malloc(sizeof(int) * 10);
p = (int *)realloc(p, 100);

It is mostly considered bad practice in C to cast the return value of malloc(), calloc() and realloc(). The void * return value will automatically be converted to the appropriate pointer type, unless the code is compiled as C++ code, which is an estranged cousin of C.

More importantly, it does not make sense to realloc the pointer to a hardcoded size 100. The number of ints that will be accessible in the reallocated array will depend on the actual size of the int type, which can vary from one system to another. Indeed on some architectures, 100 is not even a multiple of sizeof(int). The author might have meant to write p = realloc(p, sizeof(int) * 100); or p = realloc(p, sizeof(*p) * 100);, and more context around the fragment could help understand the intent... As written, it is very likely a bug, for multiple reasons.

Upvotes: 3

R.. GitHub STOP HELPING ICE
R.. GitHub STOP HELPING ICE

Reputation: 215507

You are correct that directly assigning the return value of realloc to your only copy of the original pointer is a bad practice. Not only do you lose the ability to free the memory if realloc failed (I would call this the lesser issue); you also lose the data that your pointer pointed to.

For some programs this may not matter (e.g. if you're just going to terminate and abort the entire operation on allocation failures, which may or may not be acceptable practice, and which is a whole topic in itself) but in general you need to first store the result of realloc to a separate temp variable, and only overwrite the original pointer variable after you check that it succeeded.

Upvotes: 7

Related Questions