25

Should I free char* variables when they were initialized using string literals? To me, the syntax would lead me to the assumption that they are only stack-allocated, but this example showed me, they are not.

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

static char* globalBuffer;

typedef struct Container {
    char* buffer;
} Container;

Container* Container_new(char* buffer) {
    Container* container = malloc(sizeof(Container));
    container->buffer    = buffer;
    globalBuffer         = buffer;
    return container;
}

void Container_print(Container* container) {
    if (container->buffer != NULL) {
        printf("%s", container->buffer);
        printf("\n");
    }
    else {
        printf("Container contains a NULL-buffer.");
    }
}

Container* stage() {
    Container* container = Container_new("Test-string.");
    Container_print(container);
    return container;
}

int main() {
    Container* container = stage();
    Container_print(container);

    free(container);
    Container_print(container); // I know, this results in undefined behaviour

    printf(globalBuffer);
    printf("\n");

    return 0;
}

I get the following output:

C:\Users\niklas\Desktop>gcc char_test.c

C:\Users\niklas\Desktop>a.exe
Test-string.
Test-string.
­6>
Test-string.

C:\Users\niklas\Desktop>

So, the char* initialized with string-literals does still exist, even it got out of scope.

So, my question, should I free such char* pointers? Would this be the correct main()?

int main() {
    Container* container = stage();
    Container_print(container);

    free(container->buffer);    // NEW
    free(container);
    Container_print(container);

    printf(globalBuffer);
    printf("\n");

    return 0;
}
Community
  • 1
  • 1
Niklas R
  • 16,299
  • 28
  • 108
  • 203
  • 1
    BTW - Your `printf(globalBuffer)` and `printf(container->buffer);` will give you jip if they contain the % character. – Ed Heal Feb 29 '12 at 18:32
  • 1
    Sometimes a bit of meta reasoning can help: do you really believe a fundamental concept such as string literals can only be used correctly if accompanied by cleanup code? Surely not. – Kerrek SB Feb 29 '12 at 18:41
  • NO, you may not do that. You use free() only with memory that's been dynamically allocated with malloc( ), calloc(), or realloc( ). – Pete Wilson Feb 29 '12 at 18:56
  • 2
    To clarify, string literals are not allocated on the stack, either. They're allocated statically, meaning they're baked into the program data and loaded into memory when the program is loaded. All pointers to the strings are pointers to that location in the program data. They are neither stack nor heap. If they were allocated on the stack, then you would not be able to return them from functions safely. – zmccord Feb 29 '12 at 21:05

2 Answers2

41

You shall never free() memory that you did not malloc()ed.

The way the compiler implements string-literals is not your business: it's an implementation detail. You can free() a pointer to memory that you allocated using malloc(), and only those, or you are risking the life of your system.

Ideally, malloc() calls and free() calls should appear on the same "design level" (inside the same implementation file for the same module for example), and they should match perfectly: one free() for each malloc(). but that's not always possible.

(Note that some library allocates blocks of memory, returns pointers to those blocks, and instructs you to free them. in this case, you are allowed to free those pointers, but that is a bad design practice from the people who created the library.)

nsg
  • 10,510
  • 4
  • 21
  • 32
Adrien Plisson
  • 22,486
  • 6
  • 42
  • 73
  • 1
    Now that I look it up, it doesn't look like it's part of the standard at all. It's in POSIX. – Mysticial Feb 29 '12 at 18:38
  • 1
    @Mysticial and also why I say "don't use `strdup`" every time and every place I see it – Seth Carnegie Feb 29 '12 at 18:38
  • 2
    Well, if you're going to complain about `strdup`, you could complain about `calloc` and `realloc` too... – jamesdlin Feb 29 '12 at 19:08
  • 5
    If it's bad design for libraries to give you pointers and tell you to free them when you don't need them anymore... what is the right way of handling freeing memory malloc'd by libraries? – Attila Szeremi Sep 20 '13 at 15:59
  • 5
    @SzerémiAttila : the library should provide you with a function call to free the memory it has allocated. this ensures that the correct function is called to free the pointer (depending on the runtime used and the way the library is linked, `malloc()` and `free()` may not be the same inside or outside the library) – Adrien Plisson Sep 21 '13 at 16:04
37

String literals are stored in such a way that they're available for the lifetime of the program; if you write

char *ptr = "This is a test";

all that's written to ptr is the address of the string literal "This is a test". Even if the ptr variable goes out of scope, the string literal continues to exist in its own section of memory, which is not the same section used by malloc (at least, not at a logical level). Note that multiple instances of the same string literal may resolve to the same location; IOW, given

char *p0 = "This is a test";
char *p1 = "This is a test";

p0 and p1 may both contain the same address (it's up to the compiler whether multiple occurrences of string literals are mapped to the same location or not).

When you call Container_new, all you're doing is copying an address to container->buffer and globalBuffer; both wind up pointing to the same thing that exists independently of either of them. free-ing container doesn't affect the string literal that container->buffer points to, so printf(globalBuffer); still displays "Test-string.".

In summary, you should not call

free(container->buffer);

for this particular program, since you didn't assign the result of a malloc, calloc, or realloc call to it.

If, OTOH, you had written Container_new as

Container* Container_new(char* buffer) 
{
  Container* container = malloc(sizeof(Container));
  container->buffer    = malloc(strlen(buffer) + 1);  // Allocate memory to 
  if (container->buffer)                              // store a *new* instance
  {                                                   // of the input string. 
    strcpy(container->buffer, buffer);                // This will need to be 
  }                                                   // freed before freeing
  globalBuffer         = buffer;                      // the container
  return container;
}

then you would need to free container->buffer before freeing container.

John Bode
  • 119,563
  • 19
  • 122
  • 198