1

I am facing a problem where I get double free or corruption error when I try to free up a memory, outside of the function in which I had allocated that block of memory, even though I had passed the pointer to the memory block to a pointer outside of that function.

My code is such that in the main() function, I make a call to a function with the definition char * reverseComplement(char * pattern); as such:

char * rev = reverseComplement(dna_input);

where dna_input is a char pointer to a block of allocated memory that was allocated within main(). Within the reverseComplement() function, there is a line where I allocate memory and then return the pointer to that block of memory at the very end.

...
...
char * revcomplpattern = (char *)malloc(strlen(pattern));
...
...
return revcomplpattern;

One reason I thought could be causing the problem is that after the reverseComplement() function finished executing, its stack gets torn down so I would lose the access to the memory on the heap. But this shouldn't be because I had passed over the handle of the heap-allocated memory over from revcomplpattern which was residing in reverseComplement() over to rev which is residing in main(). So free(rev) in main() should do the job.

I don't know what I might be doing wrong here, and any help is greatly appreciated!

Shiro
  • 2,610
  • 2
  • 20
  • 36
AKKA
  • 165
  • 4
  • 15
  • 6
    `malloc(strlen(pattern))` --> `malloc(strlen(pattern)+1)` – BLUEPIXY Jul 04 '16 at 16:49
  • It's not the best of habits to allocate memory in a function and them make it the caller's responsibility to call `free()`. – babon Jul 04 '16 at 16:53
  • 1st of all, remove the cast, then see what the compiler tells you. – alk Jul 04 '16 at 17:00
  • 1
    There's nothing wrong with what you've described so far (although @BLUEPIXY may have spotted a second problem). One approach to solving problems like this is to repeatedly reduce your code, shrinking it to the smallest reproducible case, in order to find what line of code triggers the problem. That should get you closer to figuring out what's going on (or at least being able to ask the next question). That's how we used to solve problems like this before StackOverflow was created ;-) Good luck! – jdigital Jul 04 '16 at 17:06
  • Can't see check for NULL after `malloc`. Also `strdup` would be more expressive –  Jul 04 '16 at 17:36
  • Maybe some important pointer arithmetics is omitted? –  Jul 04 '16 at 17:38
  • One thing you can do (especially if you're not yet fully familiar with dynamic memory allocation/deallocation): Whenever you `free()` some pointer, set it to NULL - This at least gets rid of multiple `free`s. Other than that, use tools like *valgrind* to check for heap corruption and memory leaks. – tofro Jul 04 '16 at 17:53
  • @alk I removed the cast and compiled, nothing, the compiler didn't tell me anything, and even with the casts, the compiler never told me anything... – AKKA Jul 05 '16 at 03:00
  • @tofro Following your suggestion, I used Valgrind, and when I had actually did the `free()` Valgrind correctly tells me that I do not have any leaks. But the double free error still persists! When I remove the `free()` and the error gets eliminated, Valgrind tells me then that I have one block of memory leaked! What's going on?!?! – AKKA Jul 05 '16 at 03:01

3 Answers3

0

...where dna_input is a char pointer to a block of allocated memory that was allocated within main(). Within the reverseComplement() function, there is a line where I allocate memory and then return the pointer to that block of memory at the very end.

Your idea tells me that you are allocating memory and somehow using that memory in reverseComplement() function but in the function, you're allocating another block of memory. I'm not sure why.

...a problem where I get double free or corruption error...

You need to make sure all allocated blocks of memory in your program are freed by the time your program exits for any reason. If not, then you'll have a memory leak.

If you must allocate memory within a function (which I don't recommend), then you can use code like this:

char *getnewblock(int size)
{
    char *secondblock=malloc(size);
    return secondblock;
}

int main()
{
    char *myblock = malloc(1000);
    char *myblock2 = getnewblock(5000);
    free(myblock);
    free(myblock2);
    return 0;
}

The above program allocates 1000 bytes of memory then calls a function to allocate 5000 bytes of memory then frees both blocks of memory.

What I really recommend you do is allocate memory only in one function then use the same memory block in child functions like so:

void insertfirstchar(char *block,char onechar)
{
   block[0]=onechar;
}

int main()
{
    char *myblock = malloc(1000);
    insertfirstchar(myblock,'A');
    free(myblock);
    return 0;
}

In the above program, 1000 bytes are allocated, then a function is called to make the first byte of the memory block the letter "A" then the memory is freed. This last program can be used as your starting point.

Mike -- No longer here
  • 2,064
  • 1
  • 15
  • 37
  • Yes that's what I thought of doing too, but that'll make my `reverseComplement()` function have to take in an additional argument which I'd rather not do. Now I'm curious though, in professional projects, do they follow the practice you've suggested, or can my style be accepted? – AKKA Jul 05 '16 at 03:18
0

Ok guys I just got this all figured out! So sorry for the trouble! As it turns out, the error was not because of the way I was freeing my allocated memory, but at the malloc() call.

It turns out that what I did when I allocated

char * revcomplpattern = (char *)malloc(strlen(pattern));

was immediate followed by these lines

int strend = strlen(pattern)/sizeof(char) -1 ;
revcomplpattern[strend+1] = '\0';

where my goal was to ensure that revcomplpattern would be made into a string as I inserted a null termination at its end. Turns out that in indexing [strend+1], I was actually touching an unallocated position in memory! After correcting the malloc() call with an additional space for one more character

char * revcomplpattern = (char *)malloc(strlen(pattern) + 1 * sizeof(char));

The call to the free(rev) does not throw an error! Everything works fine now and Valgrind shows no errors! Special shoutout to @tofro and @jdigital for your advising. When I used valgrind it started telling me there was an error in strlen and malloc which I couldn't understand at first until I narrowed it down to that line and gave a hard look at it and realized what was wrong.

Man, I feel that GCC gives very cryptic error messages that don't help much. I'm actually puzzled now, if anyone can answer, why my mistake DID NOT throw a SEGFAULT instead? I thought this would cause a segfault since I was touching an unallocated portion of memory???

Nonetheless, thank you everyone for your help!

AKKA
  • 165
  • 4
  • 15
  • `sizeof(char)` equals `1` by definition. If it weren't this `malloc(strlen(pattern) + 1 * sizeof(char));` would fail, as it should be `malloc((strlen(pattern) + 1) * sizeof(char));` – alk Jul 06 '16 at 07:07
  • And still, if this really *is* C code just remove all those casts to `malloc()`, as it's not needed, nor recommended in any way. This is different from C++. – alk Jul 06 '16 at 07:08
  • And finally: Writing out of the bounds of valid memory in C cause *Undefined Behaviour*. From this moment on *anything* could happen, with "*anything*" being a segmentation violation exception or simply nothing or ... or ... – alk Jul 06 '16 at 07:11
  • @alk thank you for pointing out that it should be `malloc((strlen(pattern) + 1) * sizeof(char));`! But if say `sizeof(char)` was not `1`, then it would be something larger, say `4`, then there is no harm right? Its just that I would be allocating more space than I needed? – AKKA Jul 10 '16 at 03:55
  • @alk Noted about the casting of `malloc()`. Its something that I've seen on many other discussions throughout stackoverflow. But the funny thing is that, this is how we've been taught in school! Like this is what they've been teaching in our introduction to programming and computing systems class. What's wrong with casting `malloc()`? – AKKA Jul 10 '16 at 03:56
  • @alk Thank you for your last explanation about _undefined behavior_. I understand that now. – AKKA Jul 10 '16 at 03:57
  • "*What's wrong with casting ...*" Do you do `int i = 1; long l = (long) i;`? Probably not, but let "implicit conversion" do its **well defined** work. – alk Jul 10 '16 at 18:37
  • "*What's wrong with casting ...*"^2: Also please read here: http://stackoverflow.com/q/605845/694576 and/or here: http://stackoverflow.com/q/1565496/694576 – alk Jul 11 '16 at 07:01
-1

Maybe you are corrupting memory before returning control to main()? Freeing your "valid" block of memory could be failing because your malloc heap is already corrupted. Alternatively, maybe some error codepaths in reverseComplement() already free this memory?

Liam Finnie
  • 169
  • 1
  • 2
  • 2
    "*Maybe, ... maybe ...*", maybe this answer would have suited better as a comment. – alk Jul 04 '16 at 16:59