Search code examples
cmemory-managementcrashfree

Why does free() cause a program crash in this code?


I've already searched for an answer to this, but so far haven't come up with anything that answers the specific question, so asking here.

I have some C code that dynamically allocates space for a string and copies in the contents of a buffer that's sat in a local variable. This dynamically allocated string is then stored in a struct and used elsewhere in the program.

Now, when I'm done with the struct, I'm trying to be a good citizen and deallocate the string, however when I do so, without fail, the program crashes. As far as I can tell, I'm passing in the correct pointer to free(), and if I omit the call to free() then I've confirmed that I'm leaking memory, so what am I doing wrong?

Here's the gist of the code.

typedef struct Token {
    size_t length;
    const char* lexeme;

    // Other members
} Token;

// Original string is stored in a stack allocated static buffer:
static char buffer[1024];

// ... stuff here to fill buffer

// Then we generate the dynamic copy of the buffer contents
char* formattedMessage = realloc(NULL, sizeof(char) * strlen(buffer));

// Token is dynamically allocated as needed:
Token* token = realloc(NULL, sizeof(Token));
token->lexeme = strcpy(formattedMessage, buffer);
token->length = strlen(formattedMessage);

// ... Then we do other stuff (which all works, the message is stored in the struct and all is well)

// When we're done we call:
token->lexeme = free((void*)(token->lexeme));

// Which is where the program crashes.

Note that in the actual code, I do test that token->lexeme is a valid pointer to a string and checking the memory dump as I'm tracing through shows that the address being passed into free() is the correct address for the string.

Tearing my hair out here.


Solution

  • strcpy(a, b) needs a to point to memory of at least size strlen(b) + 1. realloc(NULL, sizeof(char) * strlen(buffer)) allocates one less than needed.

    free() does not return anything. I would not expect token->lexeme = free((void*)(token->lexeme)); to even compile.


    Easy enough to fix, yet with the new C23 making strdup() part of the standard library, consider using that to allcate and copy a string - or roll your own.

    // Unnecessary casts removed
    
    char* formattedMessage = strdup(buffer);
    if (formattedMessage == NULL) {
      Handle_OutOfMemeory(); // TBD code
    }
    
    Token* token = realloc(NULL, sizeof token[0]);
    if (token == NULL) {
      Handle_OutOfMemeory(); // TBD code
    }
    token->lexeme = formattedMessage;
    token->length = strlen(formattedMessage);
    
    
    // ... Then we do other stuff 
    
    // When we're done we call:
    free(token->lexeme)
    free(token);
    

    Style: malloc(size); is much more common than realloc(NULL, size);.