-1

I wondered if I have a memory leak, I don't know whether I should use free() when realloc fails or it does that automatically.

for example, I have a dynamic array of chars, each time I'm reaching the end of it from input, I'm adding more memory using realloc. I'm saving the last address just in case the realloc fails.

int cntan = 0;
char *p = (char *)malloc(MEM_SIZE * sizeof(char));
char *q = p; /*backup pointer*/
int c;
long current = 0, last = MEM_SIZE - 1;
while ((c = getchar()) != EOF)
{
    if (current >= last) /* if reached last bit */
    {
        q = p;
        p = (char *)realloc(q, last + (MEM_SIZE * sizeof(char))); /* attempting to add more bits */
        if (p == NULL)
        {
            printf("Memory allocation failed, printing only stored values \n");
            return q;
        }
        last += MEM_SIZE;
    }

    p[current] = c;
    ++current;
    if (isalnum(c)) /* checking if current char is alpha-numeric */
        cntan++;
}

in this example, I wonder I should free(p) if p==NULL

whole code if someone interested: Pastebin

Sheilem
  • 59
  • 5
  • Does this answer your question? [Proper usage of realloc()](https://stackoverflow.com/questions/21006707/proper-usage-of-realloc) – Paul Hankin Apr 23 '22 at 15:28
  • 2
    from the [`realloc` man page](https://linux.die.net/man/3/realloc): "If `realloc()` fails the original block is left untouched; it is not freed or moved.". So it's up to you how to handle the failure, but yes, the original block is still valid. If you want to clean up properly you need to `free` it. – yano Apr 23 '22 at 15:30

2 Answers2

3

You should free() the original pointer, not the one returned by realloc(). Passing NULL to free() won’t do anything.

To clarify: the pointer returned by realloc() on success may not be the same as the original pointer.

Anonymous
  • 66
  • 4
  • I did free(), after I used q or p in the main function, I returned it through a function to the main, and before main terminates I free(q) if realloc fail once (as seen in the code here, I return q if fails) or free(p) if realloc doesn't fail at all until EOF – Sheilem Apr 23 '22 at 15:36
1

Other problems with code:

Length unknown

return q; returns the pointer, yet the caller lacks information about how big an allocation (current).

Conceptually wrong size calculation

Scaling by sizeof(char)applies to the sum. Makes no difference in this case as sizeof char is always 1.

// last + (MEM_SIZE * sizeof(char))
(last + MEM_SIZE) * sizeof(char)

Missing malloc() check

Code checks the return value of realloc(), but misses checking malloc().

Note: cast not needed. Consider:

// p = (char *)realloc(q, last + (MEM_SIZE * sizeof(char)));
p = realloc(q, sizeof q[0] * (last + MEM_SIZE));
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • How should i fix the length unknown of the caller? it just points the pointer in the caller to the other address, isn;t it? – Sheilem Apr 23 '22 at 20:43
  • @Sheilem 3 pieces of info need returning: allocation pointer, amount allocated, error flag. Many ways to do this. Usually best to start with a definition of what the function does - a job for OP. – chux - Reinstate Monica Apr 23 '22 at 20:47
  • why would it need to return the allocation pointer and the amount allocated? – Sheilem Apr 23 '22 at 20:52
  • @Sheilem Posted code had `return q;` so apparently the caller needed the allocation pointer. As the function's goals are unlisted, its true needs remains a matter of conjecture. – chux - Reinstate Monica Apr 23 '22 at 20:56