Freeing an invalid pointer?

Question

void resolve_paths(char* inputdir)
{
    char** tokens = malloc(64*sizeof(char*));
    char* ptr;
    char* slash = "/";
    int i = 0;

    ptr = strtok(inputdir,slash);
    while(ptr != NULL){
        if(strcmp(ptr,"~") == 0){
            ptr = getenv("HOME");
        }
        tokens[i] = (char*)calloc(64,sizeof(char));
        tokens[i] = ptr;
        printf("token[%i] = %s\n",i,tokens[i]);

        i++;
        ptr = strtok(NULL,slash);
    }
    int j;
    printf("freeing each element of tokens\n");
    for(j = 0; j < i; j++){
        printf("freeing token[%i]\n",j);
        free(tokens[j]);
    }
    free(tokens);
    puts("leaving resolve_paths\n");
    return;
}

My Output:

before call
token[0] = a
token[1] = b
token[2] = c
freeing each element of tokens
freeing token[0]
freeing token[1]
Error in `./a.out': free(): invalid pointer: 0x00007ff96dca12b2 ***

I guess I simply don't understand how malloc/calloc and free work. Why does this code run into a seg fault?

Will say `if(ptr == NULL){ /*show an error and exit*/ } ` be a sufficient check? — Connal Sumlin, Sep 05 '15 at 17:34
Do you allocate memory to ptr ? No , check those to whom you allocate memory. — ameyCU, Sep 06 '15 at 02:40

score 2 · Answer 1 · answered Sep 05 '15 at 17:27

Your are going wrong at tokens[i] = ptr;. You are assigning a pointer with that, not (as you probably expected) copying a string into the allocated memory, so you are freeing something you haven't actually allocated later on.

You have to use strcpy/strncpy for copying ptr into tokens[i]. Use something like strncpy(tokens[i],ptr,63) instead of tokens[i] = ptr;.

score 0 · Answer 2 · edited May 23 '17 at 12:22

0

As far as answering how malloc(),calloc() work, go here: How do malloc() and free() work? , excellent answers for your question.

edited May 23 '17 at 12:22

Community

1
1

answered Sep 05 '15 at 17:27

TimLayne

124
2
13

score 0 · Accepted Answer · answered Sep 05 '15 at 17:29

0

tokens[i] = (char*)calloc(64,sizeof(char));
tokens[i] = ptr;

This code doesn't do what you think it does. First you allocate 64 bytes and store the address in tokens[i], then you replace that address with ptr (thus losing the just allocated one which leaks away).

Probably you meant something like

tokens[i] = calloc(64,sizeof(char));
strncpy(tokens[i], ptr, 64);

which allocates the memory and then copy the string pointed by ptr into it. Mind that you don't need to cast the result of calloc or malloc in C since a void* is implicitly convertible to any other pointer.

In the end you try to free(tokens[i]) but the address contained is just a local address of inputdir adjusted by strtok, so it's not a dynamically allocated address which you can correctly free, that's why you get the error.

answered Sep 05 '15 at 17:29

Jack

131,802
30
241
343

Very helpful, thank you! Also, would changing the 2nd bit of code to `tokens[i] = calloc(strlen(ptr),sizeof(char)); strncpy(tokens[i], ptr, strlen(ptr));` be beneficial? If so, should I do it this way, or is it unneccesary? – Connal Sumlin Sep 05 '15 at 17:38
No, it's an error. You must limit `strncpy` to the size of the buffer, not on the size of the source string. By using `strlen(ptr)` you basically don't use any limit and you can find the problem you are trying to avoid, if `strlen(ptr) > 64` you overflow over the length of `tokens[i]` – Jack Sep 05 '15 at 17:40
@Jack: You should carefully read the strncpy manpage before recommending its use. At least read the **warning**. http://linux.die.net/man/3/strncpy – rici Sep 05 '15 at 18:38
1

@Connal: I recommend the simple `tokens[i] = strdup(ptr);`. If you don't have strdup, `tokens[i] = malloc(strlen(ptr) + 1); strcpy(tokens[i], ptr);` – rici Sep 05 '15 at 18:47

Freeing an invalid pointer?

3 Answers3