CODEWITHSUNDEEP
cvalgrind
user2266603
user2266603

Reputation: 103

Valgrind Address is on Thread's 1 stack

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!

Upvotes: 4

Views: 14122

Answers (3)

Christopher Schneider
Christopher Schneider

Reputation: 3915

This is solved, but I wanted to clarify something since I was getting lots of "Invalid write of size n" errors in my Valgrind logs. I thought I was using too much stack space, but that wasn't the case at all.

If you call fprintf and direct the output to stderr, all those calls will show in Valgrind. And man, will they fill up your log. One fprintf call generated around 300 of these errors.

e.g.

    fprintf(stderr,"I want to print %d and %d", num1, num2);

Will fill up your Valgrind logs. I was worrying that something was severely wrong and I couldn't find my problem. As it turns out, there were no problems and I just wasted the past 45 minutes.

Upvotes: 1

Charlie Burns
Charlie Burns

Reputation: 7044

Try replacing

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

with

tk->sep_set = strdup(separators);
tk->str = strdup(ts);

Upvotes: 2

unwind
unwind

Reputation: 399863

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).

Upvotes: 4

Related Questions