Valgrind Address is on Thread's 1 stack

Question

I can't figure out for the life of me what I'm doing wrong.

T* tokenizer = create(argv[1], argv[2]);
destroy(tokenizer);

Here is the structure:

struct T_
{
    char *sep_set;
    char *str;
    char *token;
    int *last_index;
};
typedef struct T_ T;

Here is the create function:

T *create(char *separators, char *ts)
{
    T *tk = malloc(sizeof(struct T_));
    tk->sep_set = malloc(sizeof(char)*strlen(separators));
    tk->str = malloc(sizeof(char)*strlen(ts));
    tk->last_index = malloc(sizeof(int));
    tk->sep_set = separators;
    tk->str = ts;
    *tk->last_index = 0;
    return tk;
}

void destroy(T *tk)
{
    free(tk->sep_set);
    free(tk->str);
    free(tk->last_index);
    free(tk);
}

My error is:

==12302== Invalid free() / delete / delete[] / realloc()
==12302==    at 0x4C273F0: free (vg_replace_malloc.c:446)
==12302==    by 0x400649: destroy (t.c:58)
==12302==    by 0x40088C: main (t.c:145)
==12302==  Address 0x7ff0006e7 is on thread 1's stack
==12302== 
==12302== Invalid free() / delete / delete[] / realloc()
==12302==    at 0x4C273F0: free (vg_replace_malloc.c:446)
==12302==    by 0x400659: destroy (t.c:59)
==12302==    by 0x40088C: main (t.c:145)
==12302==  Address 0x7ff0006ec is on thread 1's stack

Lines 58 and 59 are

free(tk->sep_set);
free(tk->str);

Any help would be much appreciated!

Answer 1

Your grasp of strings in C seems to be failing you.

This:

tk->sep_set = malloc(sizeof(char)*strlen(separators));
tk->sep_set = separators;

is wrong, it overwrites the pointer returned by malloc() with the separators argument pointer, dropping the handle to the memory which is leaked. The wrong pointer is then passed to free(), which is invalid.

It should be:

tk->sep_set = strdup(separators);

if you have it, else:

if((tk->sep_set = malloc(strlen(separators) + 1)) != NULL)
    strcpy(tk->sep_set, separators);

Some points:

1. You must add 1 to the length to make room for the '\0' terminator.
2. You don't need to "scale" by sizeof (char), that's guaranteed to be 1 so it's just clutter.
3. You must check that malloc() doesn't fail.
4. You must copy strings with strcpy().

The same is true for the str field (with the ts argument).