7

I'm trying to learn a couple things (just as a hobby) and trying to learn to use Valgrind. However this doesn't seem to make sense to me. It seems Valgrind is saying that bytes are lost when I'm allocating them with calloc before I even use anything! Can someone explain what is going on here and why the second program worked? I compiled the programs in debug mode in Eclipse and ran Valgrind on the debug executable.

Here's the program:

1 #include <stdlib.h>
2 #include <stdio.h>
3 #include <string.h>
4
5 int main(void) {
6
7     char* origstr = calloc(37, sizeof(char*));
8     char* newsubstr = calloc(9, sizeof(char*));
9
10    origstr = "TheQuickBrownFoxJumpedOverTheLazyDog";
11
12    strncpy(newsubstr, origstr + 8, 8);
13    printf("SubString is: %s\n", newsubstr);
14
15    free(newsubstr);
16    free(origstr);
17    return 0;
18 }

And here's what Valgrind gives me:

$ valgrind --tool=memcheck --leak-check=full ./test
==25404== Memcheck, a memory error detector
==25404== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==25404== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==25404== Command: ./test
==25404== 
SubString is: BrownFox
==25404== Invalid free() / delete / delete[] / realloc()
==25404==    at 0x4C29E90: free (vg_replace_malloc.c:473)
==25404==    by 0x400665: main (test.c:16)
==25404==  Address 0x4006f8 is not stack'd, malloc'd or (recently) free'd
==25404== 
==25404== 
==25404== HEAP SUMMARY:
==25404==     in use at exit: 296 bytes in 1 blocks
==25404==   total heap usage: 2 allocs, 2 frees, 368 bytes allocated
==25404== 
==25404== 296 bytes in 1 blocks are definitely lost in loss record 1 of 1
==25404==    at 0x4C2AD10: calloc (vg_replace_malloc.c:623)
==25404==    by 0x4005FC: main (test.c:7)
==25404== 
==25404== LEAK SUMMARY:
==25404==    definitely lost: 296 bytes in 1 blocks
==25404==    indirectly lost: 0 bytes in 0 blocks
==25404==      possibly lost: 0 bytes in 0 blocks
==25404==    still reachable: 0 bytes in 0 blocks
==25404==         suppressed: 0 bytes in 0 blocks
==25404== 
==25404== For counts of detected and suppressed errors, rerun with: -v
==25404== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

If I remove the two free() statements, here's what Valgrind gives me:

$ valgrind --tool=memcheck --leak-check=full ./test
==25597== Memcheck, a memory error detector
==25597== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==25597== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==25597== Command: ./test
==25597== 
SubString is: BrownFox
==25597== 
==25597== HEAP SUMMARY:
==25597==     in use at exit: 368 bytes in 2 blocks
==25597==   total heap usage: 2 allocs, 0 frees, 368 bytes allocated
==25597== 
==25597== 72 bytes in 1 blocks are definitely lost in loss record 1 of 2
==25597==    at 0x4C2AD10: calloc (vg_replace_malloc.c:623)
==25597==    by 0x4005BF: main (test.c:8)
==25597== 
==25597== 296 bytes in 1 blocks are definitely lost in loss record 2 of 2
==25597==    at 0x4C2AD10: calloc (vg_replace_malloc.c:623)
==25597==    by 0x4005AC: main (test.c:7)
==25597== 
==25597== LEAK SUMMARY:
==25597==    definitely lost: 368 bytes in 2 blocks
==25597==    indirectly lost: 0 bytes in 0 blocks
==25597==      possibly lost: 0 bytes in 0 blocks
==25597==    still reachable: 0 bytes in 0 blocks
==25597==         suppressed: 0 bytes in 0 blocks
==25597== 
==25597== For counts of detected and suppressed errors, rerun with: -v
==25597== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Now, if I run this program:

1 #include <stdlib.h>
2 #include <stdio.h>
3 #include <string.h>
4
5 int main(void) {
6
7    char* origstr;
8    char* newsubstr = calloc(9, sizeof(char*));
9
10   origstr = "TheQuickBrownFoxJumpedOverTheLazyDog";
11
12   strncpy(newsubstr, origstr + 8, 8);
13   printf("SubString is: %s\n", newsubstr);
14
15   free(newsubstr);
16
17   return 0;
18 }

It shows everything is just fine:

$ valgrind --tool=memcheck --leak-check=full ./test
==25862== Memcheck, a memory error detector
==25862== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==25862== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==25862== Command: ./test
==25862== 
SubString is: BrownFox
==25862== 
==25862== HEAP SUMMARY:
==25862==     in use at exit: 0 bytes in 0 blocks
==25862==   total heap usage: 1 allocs, 1 frees, 72 bytes allocated
==25862== 
==25862== All heap blocks were freed -- no leaks are possible
==25862== 
==25862== For counts of detected and suppressed errors, rerun with: -v
==25862== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Why is it that I can't calloc (allocate) origstr and then give it something? What if I wanted to allocate that variable and in the course of the program give it part of what's in another string variable or use it to capture the result of another function that returns a string? Would I then have to handle it like I did newsubstr?

This is a bit confusing to me so can someone explain how this works so I can understand it better?

  • You're losing (leaking) the `calloc()`ed pointer with the assignment on line 10. – Kninnug Oct 05 '15 at 16:31
  • `origstr = "TheQuickBrownFoxJumpedOverTheLazyDog";` This does _NOT_ set the contents of `origstr`; it sets `origstr` to point to a read-only section of memory that contains the string literal. The memory you allocated beforehand into `origstr` is lost, and you can't call `free` on string literals. – Colonel Thirty Two Oct 05 '15 at 16:31

3 Answers3

8
origstr = "TheQuickBrownFoxJumpedOverTheLazyDog";

By doing this you change to what origstr points to . After this origstr doesn't point to memory block allocated by calloc .

And you free memory not allocated by calloc or similar functions , thus causing error in your program.

Use strcpy to copy string to origstr -

strcpy(origstr,"TheQuickBrownFoxJumpedOverTheLazyDog");

and then you can free your pointer origstr.

ameyCU
  • 16,489
  • 2
  • 26
  • 41
  • I am using strncpy(newsubstr, origstr + 8, 8); because what I'm doing there is to get a substring of the original string (ie. "BrownFox"). I thought this would be the most easiest way to go about it (based on some things I've read). –  Oct 05 '15 at 20:04
  • @RavenLX Then maybe you could have a string literal , But that would make that constant . And also I think you are clear on `calloc` that you use large amount of memory . `calloc(37,1);` could have done it for you . – ameyCU Oct 06 '15 at 03:19
4

By assigning the string literal to origstr you don't copy the string but just change origstrs value, thus losing the pointer to calloc. freeing origstr now causes undefined behavior.

Use strcpy or strncpy instead to really store the string on the heap. But actually dropping calloc for origstr should suffice.


Notes:

  • as @LeeDanielCrocker mentioned in the comments to this answer, you probably intended to allocate space for chars, not for char*s, decreasing the size of the allocated memory drastically. You should replace the sizeof(char*) with a sizeof(char) (a.k.a. 1).
Community
  • 1
  • 1
cadaniluk
  • 15,027
  • 2
  • 39
  • 67
  • Also, he's allocating way too much memory: a 37-character string needs only 38 bytes (an extra for the terminating zero). You're allocating memory for 37 character pointers; pointers might be as big as 8 bytes each, which is why the block you're losing is so big. – Lee Daniel Crocker Oct 05 '15 at 17:32
  • The string itself is only 36 characters which is why I had used 37 (to include the idea that there is going to be a null terminator). I assumed that the null terminator is automatically put in. But maybe it isn't? I know now I should use strcpy so I should copy to it "TheQuickBrownFoxJumpedOverTheLazyDog\0"? That looked rather odd to me for some reason. –  Oct 05 '15 at 18:37
  • @RavenLX `""` is a `char[1]` because it only consists of one null byte. `strcpy` automatically copies the null byte, `strncpy` only does if `n` is big enough. – cadaniluk Oct 05 '15 at 18:40
  • I just tried it with calloc(1, sizeof(char)) and got errors because it wasn't enough space. calloc(9, sizeof(char)) gives no errors. So it appears that it does want the number of characters you expect to have in the string. Here's an example I found online that seems to make sense to me: http://www.tutorialspoint.com/c_standard_library/c_function_calloc.htm (I hope it's OK to post the link?) –  Oct 05 '15 at 20:09
  • @RavenLX Yes, of course you need to pass the number of `char`s. I just meant that `sizeof(char) == 1`. I also linked to a question concerning that fact. – cadaniluk Oct 06 '15 at 04:11
2

Because there is a memory leak. You reassign the pointer, it's actually incorrect to free() it as you have it.

To copy the contents to the allocated pointer use strcpy()

strcpy(origstr, "TheQuickBrownFoxJumpedOverTheLazyDog");

Let's see how:

  1. You request memory with calloc()

    origstring = calloc(9, sizeof(char*))
    

    this is wrong for multiple reasons

    1. You are allocating space for 9 pointers, not 9 characters.
    2. You don't really need calloc() because you will overwrite the contents immediately, use malloc().
  2. You overwrite the pointer with a string literal

    origstr = "TheQuickBrownFoxJumpedOverTheLazyDog";
    

    now you lost reference to the pointer returned earlier by calloc() and you cannot possible free() it, you should only free() pointers return by malloc()/calloc()/realloc().

The truth is, you don't need to calloc() the oristring pointer, calloc()/malloc() are not used to allow you to assign to a pointer, but to write to the memory pointed to by the pointer, or better, to point to some memory you can read/write from/to.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
  • 2
    If this "*You don't really need calloc()*" refers to `newsubstr`, it is not correct for the code shown by the OP. The `0`-initialising `calloc()` is essentially necessary, as the `strncpy()` does *not* copy a `0`-terminator. – alk Oct 05 '15 at 16:42