Freeing a null pointer returned from realloc?

Question

When I have a pointer which should repeatedly be used as argument to realloc and to save it's return value, I understand that realloc won't touch the old object if no allocation could take place, returning NULL. Should I still be worried about the old object in a construction like:

int *p = (int *)malloc(sizeof(int *));
if (p = (int *)realloc(p, 2 * sizeof(int *)))
 etc...

Now if realloc succeeds, I need to free(p) when I'm done with it. When realloc fails, I have assigned it's return NULL to p and free(p) doesn't do anything (as it is a free(NULL)). At the same time (according to the standard) the old object is not deallocated. So should I have a copy of p (e.g. int *last_p = p;) for keeping track of the old object, so that I can free(last_p) when realloc fails?

Cobelligerent

Do I cast the result of malloc – Hardej

FreeBSD proides a reallocf() function that does the same thing as realloc() but frees the existing pointer if sufficient additional memory cannot be allocated. Failed calls to realloc() are apparently a common source of memory leaks, so you should definitely handle this situation if your program isn't going to quit immediately when memory allocation fails. – Editorial

@Hardej Windows, VS C++ compiler and C code. even Unis here in the UK use it to teach C. – Cortex

you could initialize p with with NULL. realloc(NULL, size) works like malloc(size) – Jericajericho

OT: Your sizeof should be int instead of int* – Impregnate

Answer 1

So should I have a copy of p (e.g. int *last_p = p;) for keeping track of the old object, so that I can free(last_p) when realloc fails?

Basically: yes, of course.

It is generally not suggested to use the pattern you showed:

p = (int *)realloc(p, ...) 

This is considered bad practice for the reason you found out.

Use this instead:

void *new = realloc(p, ...);
if (new != NULL)
  p = new;
else
  ...

It is also considered bad practice to cast the return value of malloc, realloc etc. in C. That is not required.

Democrat

And you free p if void *new = realloc(p, ...) has failed, right? – Cobelligerent

@Cobelligerent totally depends on your program requirements and flow. Maybe you want to carry with data you've already allocated (p) with the knowledge that no more memory is available (at the moment). Maybe you want to free some of your data. Maybe you want to error out and halt execution. Maybe something else ... ? – Marx

@Student: Whether you free the memory p points depends on whether you want that memory or not. If you are going to abandon the task and stop using that memory, then call free(p). If you are going to continue the task, just using the existing memory instead of the changed allocation you want, then do not call free(p). – Lauritz

Also, do not say “free p”. You are not allocating or freeing pointers; you are allocating or freeing memory. You should not think of malloc, realloc, and free as allocating or freeing pointers, that is, the objects of pointer type. Objects of pointer type merely hold pointer values. malloc and realloc return a pointer value, and what object or objects you store it in are irrelevant to its operation or to the operation of free. What object you use to get the value that you pass to free is irrelevant; only the pointer value passed is used. – Lauritz

@Cobelligerent as others mentioned, it is up to your needs what you do in case of error. If you cannot collect more data, you may also decide to use whatever you got so far. Or you may decide that it's not worth the effort if you cannot get enough and you rather want to free all memory and terminate immediately. – Democrat

Basically, once some heap allocation has failed, it is safe to assume your program is about to get toasted since the computer is running out of RAM. It's probably not very meaningful to keep executing then, unless the program is mission-critical somehow. But then you shouldn't be using a PC in the first place... – Gribble

Answer 2

Should you save a temporary pointer?

It can be a good thing in certain situations, as have been pointed out in previous answers. Provided that you have a plan for how to continue execution after the failure.

The most important thing is not how you handle the error. The important thing is that you do something and not just assume there's no error. Exiting is a perfectly valid way of handling an error.

Don't do it, unless you plan a sensible recovery

However, do note that in most situations, a failure from realloc is pretty hard to recover from. Often is exiting the only sensible option. If you cannot acquire enough memory for your task, what are you going to do? I have encountered a situation where recovering was sensible only once. I had an algorithm for a problem, and I realized that I could make significant improvement to performance if I allocated a few gigabytes of ram. It worked fine with just a few kilobytes, but it got noticeably faster with the extra ram usage. So that code was basically like this:

int *huge_buffer = malloc(1000*1000*1000*sizeof *hugebuffer);
if(!huge_buffer) 
    slow_version();
else
    fast_version();

In those cases, just do this:

p = realloc(p, 2 * sizeof *p)
if(!p) {
    fprintf(stderr, "Error allocating memory");
    exit(EXIT_FAILURE);
}

Do note both changes to the call. I removed casting and changed the sizeof. Read more about that here: Do I cast the result of malloc?

Or even better, if you don't care in general. Write a wrapper.

void *my_realloc(void *p, size_t size) {
    void *tmp = realloc(p, size);

    if(tmp) return tmp;

    fprintf(stderr, "Error reallocating\n");
    free(p);
    exit(EXIT_FAILURE);
    
    return NULL; // Will never be executed, but to avoid warnings
}

Note that this might contradict what I'm writing below, where I'm writing that it's not always necessary to free before exiting. The reason is that since proper error handling is so easy to do when I have abstracted it all out to a single function, I might as well do it right. It only took one extra line in this case. For the whole program.

Related: What REALLY happens when you don't free after malloc?

About backwards compatibility in general

Some would say that it's good practice to free before exiting, just because it d actually does matter in certain circumstances. My opinion is that these circumstances are quite specialized, like when coding embedded systems with an OS that does not free memory automatically when they terminate. If you're coding in such an environment, you should know that. And if you're coding for an OS that does this for you, then why not utilize it to keep down code complexity?

In general, I think some C coders focuses too much on backwards compatibility with ancient computers from the 1970th that you only encounter on museums today. In most cases, it's pretty fair to assume ascii, two complement, char size of 8 bits and such things.

A comparison would be to still code web pages so that they are possible to view in Netscape Navigator, Mosaic and Lynx. Only spend time on that if there really is a need.

Even if you skip backwards compatibility, use some guards

However, whenever you make assumptions it can be a good thing to include some meta code that makes the compilation fail with wrong target. For instance with this, if your program relies on 8 bit chars:

_Static_assert(CHAR_BITS == 8, "Char bits");

That will make your program crash upon compilation. If you're doing cross compiling, this might possibly be more complicated. I don't know how to do it properly then.

Flagellum

And a call to exit will leave freeing the memory to the OS? So I can be sure that there will be no memory corruption? – Cobelligerent

@Cobelligerent Yes. You don't have to free before exiting in most cases. Unless you're coding for a very specific OS not including Win, Linux and Mac. – Flagellum

@G.Sliepen Fixed it – Flagellum

A multi-statement macro like that is problematic as well. Use the do {...} while (0) construction to ensure it can be used in a case like this: if (some_condition) REALLOCN(p, 5); But really, it's better to avoid macros completely if they are not necessary. – Ozonosphere

You oversimplify the problem. Very often I want to keep the data even if the realloc failes. I simply know that I cant add or remove any data from the allocated object. If I lose the reference to the original memory block - that data is gone. – Cortex

Answer 3

So should I have a copy of p (e.g. int *last_p = p;) for keeping track of the old object, so that I can free(last_p) when realloc fails?

Yes, exactly. Usually I see the new pointer is assigned:

void *pnew = realloc(p, 2 * sizeof(int *)))
if (pnew == NULL) {
    free(p);
    /* handle error */
    return ... ;
}
p = pnew;

Hardej

Why to free p? Maybe OP needs to keep existing data, just knowing that no new allocations are possible. I would remove it – Cortex