-2

I have this problem:

char** words = (char**)calloc(10, sizeof(char*));
for (int i = 0; i < 10; i++) {
    words[i] = (char*)calloc(100, sizeof(char));
}

I create a array of strings this way. Than in code I overwrite pointers (words[i])

char* str = calloc(strlen(temp), sizeof(char));
//fill str
words[index] = str;

And when I try to free the memory, I get HEAP CORRUPTION DETECTED error.

for (int i = 0; i < 10; i++) {
    free(words[i]);
}
free(words);

Is there any way how to do it?

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
Dominik Filyo
  • 37
  • 6
  • 1
    Welcome to Stack Overflow! [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh May 15 '16 at 17:52
  • `free` has no idea where you stored the original pointer returned by `malloc`, or that you overwrote it. It only cares that you give it a pointer value that `malloc` gave in the first place. – Weather Vane May 15 '16 at 17:55
  • 1
    `char* str = calloc(strlen(temp), sizeof(char));` --> `char* str = calloc(strlen(temp)+1, sizeof(char));` – BLUEPIXY May 15 '16 at 17:55
  • You must free the original value in `words[index]` – lost_in_the_source May 15 '16 at 17:57

4 Answers4

0

You are lucky to have got an error! Free-ing unallocated memory is just Undefined Behaviour, so it could work during all your tests and only break when you put code in production...

The rule is NEVER erase a malloc-ed pointer before it has been freed. You may have very good reasons to overwrite the words array (I do not know everything in your code) but in that case you could either use two different arrays:

  • one that you malloc and keep until you free it
  • one that you initially load from the former, proceed as you need and do not use it for freeing anything.

As a common alternative, you should free the allocated memory before reusing the pointer:

char* str = calloc(strlen(temp), sizeof(char));
//fill str
free(words[index]); // avoid the memory leak
words[index] = str; // correct because str points to another allocated buffer
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
0

Thanks! Now I know I have an error in code.

char* str = calloc(strlen(temp)+1, sizeof(char));

This actually solved the HEAP CORRUPTION DETECTED error, but I will repair the code according to your advices.

Dominik Filyo
  • 37
  • 6
0

It's quite easy actually. You just need to do this:

char** words = (char**)calloc(10, sizeof(char*));

And the to copy the string to a heap address:

words[i]=strdup(temp);

Simple as this.

strdup is a function from <string.h> that is this:

char* strdup(char* s)
{
    char* ret=malloc(strlen(s)+1);
    strcpy(ret,s);
    return ret;
}

It's pointless to allocate and then copy when you can just do it in 1 simple step.

Luis Paulo
  • 421
  • 5
  • 10
-1

First of all, by saying

 words[index] = str;

you're overwriting the previously allocated pointer, thereby losing the actual pointer (returned by initial call to calloc()), causing memory leak. You should use strcpy() to copy the content, not the pointer itself.

Something like

 strcpy(words[index], str);

should do the job for you.

Having said that,

 char* str = calloc(strlen(temp) , sizeof(char));

also looks wrong. You are probably missing the space for null-terminator while //fill str part. You may need

char* str = calloc(strlen(temp)+ 1, 1);  //sizeof char == 1, guaranteed in C
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261