Unexpected memory value change

Question

I'm new to C and I've created a small structure:

typedef struct my_struct {
  union {
    char *string;
    char *filename;
  } x;
} my_struct_t;

I have these functions to create and release my_struct instances:

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

my_struct_t *build_struct(){
  my_struct_t *obj;
  obj = malloc(sizeof(my_struct_t));
  obj->x.filename = NULL;
  obj->x.string   = NULL;
  return( obj );
}

void free_all(my_struct_t *obj){
  char *string = obj->x.string;
  char *filename = obj->x.filename;
  if (string) free(string);
  if (filename) free(filename);
  free(obj);
}

However, when I can't use them successfully. Here's my test program:

void modify_struct(my_struct_t *obj, const char *value){
    char *temp = malloc(strlen(value));
    memcpy(temp,value,strlen(value));
    obj->x.filename = temp;
}

int main(){
  my_struct_t *obj = build_struct();
  modify_struct(obj,"invented string");
  free_all(obj);
  return 0;
}

I dynamically allocate the struct setting both string, and filename to NULL. I then change the value of filename, before freeing the memory.

During the call to free_all(), it seems that string is not NULL any more so both 'if' conditions succeed, and the program attempts to free a non-allocated memory.

Why does this happen, even though I never touched string? And how can I fix it?

Answer 1

Rule of thumb: You only free memory which was allocated by allocator functions (malloc() and family).

Quoting C11, chapter §7.22.3.3, (emphasis mine)

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

In your case, you attempt to free obj->x.filename which is a pointer to a string literal, was not returned by allocator function. So, passing that to free, your code invokes undefined behavior.

That said, the syntax

 if (string) {
      free(string);
 }

does not make much sense, as passing a NULL pointer to free() is perfectly acceptable.