A few points to make:
I can't see how you use realloc()
in your code, but if you're using it like this, it's wrong:
variable = realloc(variable, amount);
When it's unable to allocate more memory, realloc()
returns NULL
but leaves the original pointer unaltered. In the above line, this means that variable
is NULL
and we've lost access to the memory it pointed to, but that memory hasn't been freed. The correct idiom is this:
void *tmp = realloc(variable, amount);
if(tmp)
{
// success! variable invalid, tmp has new allocated data
variable = tmp;
}
else
{
// failure! variable valid, but same size as before, handle error
}
The reason you should use the second one is because, with realloc()
, failure is bad, but is quite recoverable in many situations, unlike malloc()
where failure usually means "stop everything and die."
This is a more contentious issue, but it is questionable whether or not you should cast the return value of malloc()
and realloc()
like you do. Consider:
// functionally identical in C
char *a = malloc(10);
char *b = (char *)malloc(10);
In C++, the cast must be made, because in C++ void *
cannot be implicitly converted to another pointer type. (I think this is a language mistake, but it's not my place to judge.) If your code is C++, you should be using new
and delete
anyway. If your code is C but needs to compile with C++ compilers (for some inane reason), you have no choice but to cast. If you don't need to compile C code with C++ compilers (which is similar to having to run Ruby code in a Python interpreter), continue to the points below, which are why I think you shouldn't cast.
In C89, if a function is used without being declared, it will be implicitly declared as returning an int
. If, say, we forgot to #include <stdlib.h>
and we called malloc()
, the version without a cast would cause a compiler error (implicit casts from int
to char *
aren't allowed), while the version with the cast would (wrongly) tell the compiler "I know this sounds crazy, but cast it anyway." Most compilers will give you a warning for implicit (or incompatable) declarations of built-in functions like malloc()
, but the cast does make it harder to find.
Say you have some data:
float *array = (float *)malloc(10 * sizeof(float));
Later, you discover that you need more precision on your data, and have to make this a double
array. In the above line, you need to change no more than 3 different places:
double *array = (double *)malloc(10 * sizeof(double));
If, on the other hand, you had written:
float *array = malloc(10 * sizeof *array);
You would only need to change float
to double
in 1 place. Furthermore, always using sizeof *obj
instead of sizeof(type)
and never using casts means that a later call to realloc()
can work without any changes, while using casts and explicit type names would require finding anywhere you called realloc
and change the casts and sizeof
s. Also, if you forget, and do this:
double *array = (float *)malloc(10 * sizeof(float));
On most platforms, array
will now only be an array of 5 elements, assuming the alignment isn't off and the compiler doesn't complain that you're assigning a float *
to a double *
. Some consider the warning the compiler issues to be helpful, as it points out potentially incorrect lines. However, if we avoid sizeof(type)
and avoid casting, we can see that the lines won't be incorrect, so having the compiler draw attention to them is wasting time we could be using to program.