3

I'm a novice with valgrind, so I may do something wrong, but what am I to do if valgrind reports more frees than allocs?

Got a SSCCE here:

#include <cstring>

class something
{
protected:
    char* ptr;
public:
    something() {ptr = NULL;}
    something(const char* value) {
        ptr = new char[strlen(value)+1]; strcpy(ptr, value);
    }
    ~something() {delete[] ptr; ptr = NULL;}
};

int main()
{
    something x;
    x = "123";
    return 0;
}

which compiles fine, and runs fine too, but valgrind says

==15925== Invalid free() / delete / delete[]
==15925==    at 0x40221EA: operator delete[](void*) (vg_replace_malloc.c:364)
==15925==    by 0x8048689: something::~something() (test.cpp:12)
==15925==    by 0x80485F5: main (test.cpp:19)
==15925==  Address 0x42b7028 is 0 bytes inside a block of size 4 free'd
==15925==    at 0x40221EA: operator delete[](void*) (vg_replace_malloc.c:364)
==15925==    by 0x8048689: something::~something() (test.cpp:12)
==15925==    by 0x80485E5: main (test.cpp:18)
==15925==
==15925== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 18 from 1)
==15925== malloc/free: in use at exit: 0 bytes in 0 blocks.
==15925== malloc/free: 1 allocs, 2 frees, 4 bytes allocated.
==15925== For counts of detected errors, rerun with: -v
==15925== All heap blocks were freed -- no leaks are possible.

and I'm unsure why.
Of course I can make educated guesses - obviously the offending line is where it says x = "123"; and if you comment that out, everything is good. But why then, does the compiler think this is all right, even with -Wall -Wextra -pedantic? Did I forget a compiler switch that can tell me this program has issues?

Mr Lister
  • 45,515
  • 15
  • 108
  • 150

3 Answers3

1

x = "123" is equivalent to x = something("123"), which invokes the implicit copy-assignment operator. The temporary something is destructed, but then so is x, both of which delete [] the same raw pointer.

The solution is to follow the Rule of Three, or to use smart pointers/containers to do your memory management for you.

Kendall Frey
  • 43,130
  • 20
  • 110
  • 148
Oliver Charlesworth
  • 267,707
  • 33
  • 569
  • 680
1

You forgot to implement the Rule of three.
You should provide a copy constructor as well as the copy assignment operator.

x = "123";

Invokes the implicitly generated copy assignment operator which makes an shallow copy of the object , and once the temporary gets destroyed the destructor deallocates the allocated memory leaving your pointer member as a dangling pointer.

Community
  • 1
  • 1
Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • Definitely. But then my final question remains: are there compiler switches to `g++` that can detect these kinds of things? – Mr Lister Apr 28 '12 at 13:07
  • @MrLister: Unfortunately, no Rule of Three is not a part of the C++ standard(it should have ideally been) so the compilers don't complain about it being violated.Also AFAIK, there is no compiler extension which reports about it. – Alok Save Apr 28 '12 at 13:11
0

Your something class contains a raw pointer which is not properly managed by its (compiler-provided) copy constructor and assignment operator. Don't use raw pointers, or if you must, define your class methods more carefully. But really, don't use raw pointers.

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • You shouldn't be afraid of raw pointers. As long as you make sure that your `new`s match your `delete`s, i.e. don't allow other processes than the ones you control access to your pointers. In my example, only the constuctor can do `new` and only the destructor can do `delete`. – Mr Lister Apr 28 '12 at 13:10
  • 1
    Hey, you are the one who came here because you had a problem which would never have happened if you had used smart pointers. There is virtually no reason to use raw pointers today when you are managing object lifetimes, what with the variety available in various libraries and recently even in standard C++. That you now say *I* should not be afraid of using raw pointers when you yourself failed to use them properly is...I don't know. – John Zwinck Apr 28 '12 at 14:18
  • Oh, sorry if I offended. Maybe it's just a reaction on my behalf to forced exposure to C#, where the language holds your hand and doesn't let you do anything it thinks could hurt you. Whereas I learn far more from mistakes like this. Again, sorry. – Mr Lister Apr 28 '12 at 14:31