Trying to get my head around when to call free in C

edit It appears that this is just a case of the sample code being wrong. Thanks for clearing this up, SO.

Looking at the following code/quote from http://staff.um.edu.mt/csta1/courses/lectures/csa2060/c8a.html

//f.c    
#include <stdio.h>
#include <stdlib.h>

char *foo(char *);

main() {
    char *a = NULL;
    char *b = NULL;

    a = foo("Hi there, Chris");
    free(a);

    b = foo("Goodbye");
    free(b); 

    printf("From main: %s %s\n", a, b);
}

char *foo(char *p) {
    char *q = (char *)malloc(strlen(p)+1);
    strcpy(q, p);
    printf("From foo: the string is %s\n", q);    
    return q;
}

If free(b) is omitted, then “Goodbye” can be seen to be written to the location of “Hi there, Chris”.

I don't understand why you have to call free before using the variables being free'd in the printf() statement (indeed, in my mind it seems like free开发者_如何转开发ing up the memory first would make this fail).

Apologies if this is a repeat, but having searched/read what I could find I'm still in the dark. Code and quote are from here: http://staff.um.edu.mt/csta1/courses/lectures/csa2060/c8a.html

edit It appears that this is just a case of the sample code being wrong. Thanks for clearing this up, SO.

---

You call free() when you won't need to use the memory again.

Your printf() comes after you've freed both the strings, so you invoke 'undefined behaviour' (UB) when you try to print the strings. There is a moderate chance that you get the same address for both a and b in main(), in which case, you can only have one of the two strings stored in the space, of course. But that's still UB and anything could happen.

You should only call free() after the printf() in main().

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

char *foo(char *);

int main(void)
{
    char *a = NULL;
    char *b = NULL;

    a = foo("Hi there, Chris");
    b = foo("Goodbye");

    printf("From main: %s %s\n", a, b);

    free(a);    // Now it is safe to free the memory
    free(b); 
    return 0;
}

char *foo(char *p)
{
    char *q = (char *)malloc(strlen(p)+1);
    strcpy(q, p);
    printf("From foo: the string is %s\n", q);    
    return q;
}

---

When you call free(a), you're instructing the runtime to release the memory pointed to by the pointer variable a. By the time you get to the printf, a is no longer pointing to valid memory. By chance, b is allocated the same memory that a once had. (Then b is freed too, so neither pointer is valid.)

Printing the strings at the two invalid pointers is undefined behaviour. By chance, the memory contains the contents of the strings you copied there earlier.

---

Using a variable after it has been freed is an error. In your program you print the values after calling free() on them, which is wrong. If it works, this is by accident.

It is generally considered best practice to call malloc() and free() in the same function. In your example, this would mean that you call malloc, pass the generated buffer to foo() as parameter, print the result and call free.

---

You must be getting an output like:

From foo: the string is Hi there, Chris
From foo: the string is Goodbye
From main: 

This makes perfect sense according to your code as you have freed the variable you are going to use in the last printf statement.

I modified your code to:

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

char *foo(char *);

main() {
  char *a = NULL;
  char *b = NULL;

  a = foo("Hi there, Chris");

  b = foo("Goodbye");

  printf("From main: %s %s\n", a, b);
  free(a);
  free(b);
}

char *foo(char *p) {
  char *q = (char *)malloc(strlen(p)+1);
  strcpy(q, p);
  printf("From foo: the string is %s\n", q);
  return q;
}

The output of the above program is:

From foo: the string is Hi there, Chris
From foo: the string is Goodbye
From main: Hi there, Chris Goodbye

---

This is wrong on many levels. In foo, you're dynamically allocating space for the character string and populating it - this is good.

The point is that you now have a container (well, memory block) for each string, a and b since you called foo on both. You need that container to hold the string for the duration you wish to use it.

So, you cannot call free on a or b until you're finished using them. You call it too early (before your printf()), so you're causing undefined behavior. For all you know, the computer has reused the memory spaces for a and b before your printf even prints their contents. This could cause you to seg fault or something nasty like that.

Call free on both after your printf when you're done.

Also, set the pointers a and b to NULL so you know they're empty. Always check that your pointers have a value (non-null) before using/dereferencing them) and you'll save yourself alot of trouble.