Is it safe to use realloc?

Question

Some time ago a friend of mine told me not to use realloc because it's unsafe, but he couldn't tell me why, so I made some research on the subject and the nearest references to my doubt were:
https://buildsecurityin.us-cert.gov/bsi/articles/knowledge/coding/809-BSI.html
http://www.iso-9899.info/wiki/Why_not_realloc
I want to know if I can continue to use realloc in my code or if it's unsafe is there any other way to reallocate memory? Thanks for your attention.

Answer 1

realloc is safe in itself, but using it safely is a bit tricky -- to the point that I'd say roughly 85-90% of the code I've seen that uses it does not do so safely. The problem is that realloc returns NULL to indicate failure -- but when it does so, the pointer you supplied as input is still valid (provided you didn't resize its allocation to 0).

Therefore, you have to assign the return from realloc to the pointer you supplied as input if and only if realloc returned a non-null pointer. If it returns a null pointer, your previous pointer is valid, but the allocation has not be resized.

Also note that many people assume realloc can only fail and/or move the allocation when you enlarge the allocation. In reality, it can fail (though that's unlikely) or move the data to a different location (much more likely) even when you're reducing the allocation size.

Answer 2

Like everything in C, as long as you know what do you do, it's fine.

(Knowing what do you do includes checking for errors, don't use the old pointer,etc)

Answer 3

It's perfectly safe to use realloc. It is the way to reallocate memory in a C program.

However you should always check the return value for an error condition. Don't fall into this common trap:

p = realloc(p, new_size); // don't do this!

If this fails, realloc returns NULL and you have lost access to p. Instead do this:

new_p = realloc(p, new_size);
if (new_p == NULL)
    ...handle error
p = new_p;

Answer 4

The first of the two linked article raises two complaints above and beyond the "check the call succeeded" points already raised here.

When this is done, the old contents are discarded and left in memory somewhere. For secure memory applications where it is important to erase all traces of data, this behavior is inappropriate.

This is a valid point if you happen to be storing sensitive data (e.g. private keys, unhashed(!) passwords etc.) and want to make it harder for exploits to recover the data or other processes on the system to steal the data.

Since it moves memory around, any old pointers to that memory become invalid and could cause the program to crash or otherwise misbehave.

This point seems like nonsense to me. Their proposed solution is no better, they malloc(), copy and then free() the original which has the same net effect - the address has changed. If you wanted to avoid moving the memory you might be able to use some platform specific calls to do that, if you arranged for there to be sufficient free address space near them. If you knew a priori how much address space to reserve then you'd probably not be thinking of calling realloc() in the first place though!

If you're gambling on realloc() never moving, always growing then you've probably got bigger problems to worry about anyway and switching to malloc() + copy + free() can't possibly solve that.

---

Besides the "check your return value properly point", the most interesting point from the second article is a warning about:

Do not realloc your buffer by 1 byte at a time.

they warn:

This is guaranteed to churn your memory heap

This is a potentially valid point, but it's not a criticism of realloc() itself; the same would happen if you used malloc()+copy+free(). The real fix is to grow buffers sensibly regardless of how you grow them or better yet allocate in correct sized chunks up front.

They also have a point about

Using realloc to return memory to the system.

They're correct here in that using any size other than 0 might not actually make a return. It probably makes things no worse, but this usage still seems like an example of premature "optimisation". The fix again is to use sensible sized allocations to begin with.

Sort answer: it's not unsafe, but it's not a magical solution to all your problems either.