Friday, 11 March 2016

A frequently used incorrect realloc() idiom

While running static analysis on a lot of C source code, I keep on finding a common incorrect programming idiom used with realloc() allocation failures where a NULL is returned and the code returns with some kind of exit failure status, something like the following:

 ptr = realloc(ptr, new_size);  
 if (!ptr)   
     return -ENOMEM;  /* Failed, no memory! */  

However, when realloc() fails it returns NULL and the original object remains unchanged and thus it is not freed.  So the above code leaks the memory pointed to by ptr if realloc() returns NULL.

This may be a moot point, since the error handling paths normally abort the program because we are out of memory if can't proceed any further.  However, there are occasions in code where ENOMEM may not be fatal, for example the program may reallocate smaller buffers and retry or free up space on the heap and retry.

A more correct programming idiom for realloc() perhaps should be:

 tmp = realloc(ptr, new_size);  
 if (!tmp) {  
     free(ptr);  
     return -ENOMEM;  /* Failed, no memory! */  
 }  
 ptr = tmp;  

..which is not aesthetically pleasing, but does the trick of free'ing memory before we return.

Anyhow, it is something to bear in mind next time one uses realloc().