Search code examples
cstringmallocreallocdouble-pointer

Crashing when using free in the incorrect way


typedef char* DString;
DString dstring_initialize(const char* str);
int dstring_concatenate(DString* destination, DString source);
void dstring_delete(DString* stringToDelete);

int main(void)
{
    DString str1, str2;

    str1 = dstring_initialize("hello ");
    str2 = dstring_initialize("world");
    dstring_concatenate(&str1, str2);
    dstring_delete(&str1);
    dstring_delete(&str2);

}

DString dstring_initialize(const char* str)
{
    char* res = malloc(strlen(str) + 1);
    if (res != NULL)
    {
        strcpy(res, str);
    }
    return res;
}

int dstring_concatenate(DString* destination, DString source)
{
    DString newstr = realloc(*destination, ((strlen(*destination)+1)+ strlen((&source)+1))* sizeof *source);
    if(newstr == NULL)
    {
        printf("Error");
    }
    strcat(newstr, source);
    *destination = newstr;

    return 1;
}

void dstring_delete(DString* stringToDelete)
{
    assert(stringToDelete != NULL);
    free(stringToDelete);
    assert(*stringToDelete == NULL);
}

I suspect that my allocation is wrong or the freeing of the memory allocated is incorrect because when I debug the code, it gets stuck at the free(stringToDelete); part and then crashes. Another thing is I suspect that the realloc part is not correct. Any help would be appreciated, and if the question is terrible, feel free to comment so that I can improve my communications skills and further improve the question.


Solution

  • For the realloc, take a close look here:

     strlen((&source)+1))
    

    You're passing a char ** to strlen. What you want is:

     strlen(source)+1
    

    With the full line being:

    DString newstr = realloc(*destination, (strlen(*destination) + 1 + 
                                            strlen(source) + 1) * sizeof *source);
    

    The problem with the free is you're passing in what was allocated. You're instead passing in the address of a local variable in main. You need to dereference:

    free(*stringToDelete);
    

    Also, freeing memory doesn't set the pointer to NULL, so you should remove the assert after the free. Actually you can remove the other assert as well since passing a NULL pointer to free is well defined.