12

Is correct ways to free up memory in this code?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main( void ){

    char *string1, *string2;
    string1 = (char*) malloc(16);

    strcpy(string1, "0123456789AB");

    string2 = realloc(string1, 8);

    printf("string1 Valor: %p [%s]\n", string1, string1);
    printf("string2 Valor: %p [%s]\n", string2, string2);

    free(string1);
    free(string2);
    return 0;
}

Since the two pointers point to the same direction

Kevin
  • 1,151
  • 1
  • 10
  • 18
  • 1
    A good sanity check is to count the number of times `malloc` is called, and the number of times `free` is called. These counts should be equal. This is a necessary but not a sufficient condition for your code being good. (special cases: `strdup` counts as +1 `malloc`; `realloc` counts as +1-1 or 0) – anatolyg May 03 '16 at 20:02
  • 1
    Also, treating the memory returned from `string2 = realloc(string1, 8);` as a NUL-terminated string causes undefined behavior, since your original `string1` is longer than 8 characters. You can't truncate a C string that way and expect it to remain a string. – Andrew Henle May 03 '16 at 20:14
  • You also don't have to cast the result of `malloc`, the same way you use `realloc`. See: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – 3limin4t0r May 04 '16 at 07:46

6 Answers6

25

I think your confusion comes from the (uninspired) expression "free pointers" (you used in your post, but edited it out since). You don't free pointers. You free memory. The pointer is just telling which memory.


In your example you have: the memory obtained from malloc. string1 points to this memory. Then when you call realloc a new memory block is obtained (possibly starting at the same address, possibly not), but realloc takes care to release the old one if needed (and is therefore undefined behavior to access or free it yourself). string2 points to this new memory block.

So you have to free just the memory block obtained from realloc. string2 points to that memory.

bolov
  • 72,283
  • 15
  • 145
  • 224
8

In short, no.

Per the C Standard:

7.22.3.5 The realloc function

...

The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size. The contents of the new object shall be the same as that of the old object prior to deallocation, up to the lesser of the new and old sizes. Any bytes in the new object beyond the size of the old object have indeterminate values.

Once you call realloc(), you do not have to free() the memory addressed by pointer passed to realloc() - you have to free() the memory addressed by the pointer realloc() returns. (Unless realloc() returns NULL, in which case the original block of memory - passed to realloc() - has to be free()'d.)

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56
3

When you call realloc, either the returned pointer is the same as the original, or a new pointer is returned and the original pointer becomes invalid. In the first case, calling free on both string1 and string2 results in a double-free since the pointers are equal. In the second case, calling free on string1 is a double-free since it was already freed.

So either way you have a double-free which results in undefined behavior.

From the man page for realloc:

void *realloc(void *ptr, size_t size);

The realloc() function changes the size of the memory block pointed to by ptr to size bytes. The contents will be unchanged in the range from the start of the region up to the minimum of the old and new sizes. If the new size is larger than the old size, the added memory will not be initialized. If ptr is NULL, then the call is equivalent to malloc(size), for all values of size; if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr). Unless ptr is NULL, it must have been returned by an earlier call to malloc(), calloc() or realloc(). If the area pointed to was moved, a free(ptr) is done.

The realloc() function returns a pointer to the newly allocated memory, which is suitably aligned for any kind of variable and may be different from ptr, or NULL if the request fails.

Also from the man page for free:

The free() function frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc() or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behavior occurs. If ptr is NULL, no operation is performed.

You only need to free(string2).

dbush
  • 205,898
  • 23
  • 218
  • 273
  • Might be worth noting that if realloc returns NULL then the original pointer remains valid and must be free'd – John May 03 '16 at 20:04
  • *may* be different. But if it isn't the behavior is well defined. The lesson, I think, is to add sanity checks. – StoryTeller - Unslander Monica May 03 '16 at 20:06
  • @Trey Your [comment](if (!printlines(argc > 1 ? argv[1] : "urls.txt")) {) applies often, but not necessarily in the corner case of `realloc(string1, 0)` as it may return `NULL` and the the original pointer is free'd. – chux - Reinstate Monica May 03 '16 at 21:58
1

The realloc implicity frees the input, it may not do anything at all, but you cannot free it after to re-alloced memory. So

char *string1, *string2;
string1 = (char*) malloc(16);
....
string2 = realloc(string1, 8);  // this line implicitly frees string1

free(string1); // this is wrong !!!
free(string2);
Soren
  • 14,402
  • 4
  • 41
  • 67
1

Short answer: in your code, calling free(string2) is correct, and sufficient. Calling free(string1) is incorrect.

When you called realloc (and assuming that the call succeeded), its return value (which you stored in string2) became the one and only way to refer to the one block of memory that you have.

It may be that realloc resized your memory block "in place", meaning that the values of string2 and string1 are equal. In that case, calling free(string1) is wrong because you're freeing the same pointer twice.

It may be that realloc moved your data to a new place in the process of resizing it. In that case, the values of string2 and string1 are unequal. But in that case, after it finds a new place for your data and copies it there, realloc automatically frees the old block for you. So, again, calling free(string1) is wrong because you're freeing an already-freed pointer.

Steve Summit
  • 45,437
  • 7
  • 70
  • 103
1

Think of realloc as something equivalent to:

void *
realloc(void *old, size_t new_size)
{
    size_t old_size = magic_internal_function_that_knows_the_size_of(old);
    void *new = malloc(new_size);
    if (new == NULL)
        return NULL;
    memcpy(new, old, new_size > old_size ? old_size : new_size);
    free(old);
    return new;
}

If you have the magic function that can figure out how big an allocation is from the pointer, you can implement realloc yourself like this. malloc pretty much must have this function internally for free to work.

realloc can also do clever things like figuring out that you're reallocating to a smaller size and just free part of your allocation or figure out that you're growing your allocation and there's enough space after to fit it. But you don't need to think about those cases. Thinking that realloc is malloc+memcpy+free will not mislead you except that you need to remember that realloc failing and returning NULL means it didn't do the free.

Art
  • 19,807
  • 1
  • 34
  • 60