Use of realloc correct approach

Question

I have the following line of code, which snippet correctly uses realloc?

I have a suspicioun that the realloc call in *delete_choices line is incorrectly used is this correct?

  *delete_choices = (char*)malloc(files_max);

 if (files_size >= files_max)
    {
        files_max *= 2;
        files = (struct dirent*)realloc(files, sizeof(struct dirent) * files_max);
        *delete_choices = (char*)realloc(delete_choices, files_max);
    }

Is this the correct usage?

*delete_choices = (char*)realloc(*delete_choices, files_max);

Really confused about this

You should use a temporary pointer to receive the result from `realloc`. If it fails and returns `NULL` you've lost the original pointer so you can't continue using the data you have or free the allocation to avoid a leak. [Proper usage of realloc()](https://stackoverflow.com/questions/21006707/proper-usage-of-realloc) — Retired Ninja, May 24 '23 at 10:44
What exactly is your doubt about the usage? as `sizeof char` is defined to be 1, no need to use it to calculate the size. But in C, casting the return value of `malloc`, `realloc` etc. is not required and should be avoided as it can hide errors. Also assigning the return value directly to the initial pointer is a bad idea as Retired Ninja already mentioned. — Gerhardh, May 24 '23 at 10:53
The question cannot be answered unless you actually reveal the variable declarations. — Lundin, May 24 '23 at 10:53

Harith · Answer 1 · 2023-05-24T12:20:21.220

*delete_choices = (char*)malloc(files_max);

if (files_size >= files_max)
{
    files_max *= 2;
    files = (struct dirent*)realloc(files, sizeof (struct dirent) * files_max);
    *delete_choices = (char*)realloc(delete_choices, files_max);
}

The correct expression is:

*delete_choices = realloc (*delete_choices, files_max);

Furthermore, the above code snippet risks invoking undefined behaviour because if realloc() fails and returns a NULL pointer, you'd lose access to the original memory allocated through malloc() (assuming malloc() had succeeded), and leak memory.

Solution:

Use a temporary pointer to store the return value of realloc():

/* Yes, the usage is correct. */
char *const tmp = realloc(*delete_choices, files_max);
if (!tmp) {
    perror("realloc()");
    complain();
}
      
*delete_choices = tmp;

Note: Do not cast the return of malloc() and family. These functions return a generic void * that is implicitly converted to any other pointer type. The cast is redundant and only serves to clutter one's code.

First and foremost is using the correct expression for the address for reallocation, `*delete_choices` in both places, not `delete_choices` in one and `*delete_choices` in another. Not using a cast is not first or foremost, because the incorrect expression is an actual bug (very important), whereas the cast is a code pattern used to reduce the occurrence of bugs (less important). This “answer” does not explicitly answer the question asked. — Eric Postpischil, May 24 '23 at 12:05

score 1 · Answer 2 · answered May 24 '23 at 10:35

*delete_choices = (char*)realloc( *delete_choices, files_max ); is the correct usage.

The type that you pass must be equal to the type returned.

In your case, delete_choices appears to be of type char**, therefore *delete_choices is of type char*, so these two types are not equal.

In general: t = (T)realloc( t, n );

Use of realloc correct approach

2 Answers2