Re: [Boost-users] [Boost.Test] valgrind complains about invalid reads

Thanks for the reply Gennadiy, see inline for comments: --- On Wed, 12/16/09, Gennadiy Rozental wrote:

From: Gennadiy Rozental Subject: Re: [Boost-users] [Boost.Test] valgrind complains about invalid    reads To: boost-users@lists.boost.org Date: Wednesday, December 16, 2009, 9:32 PM Scott Banachowski writes:

...

I agree with Peter here, the destructor for test_unit

should be virtual. 

'Should' is a strong statement. Do you have anything to based it on?

Ok, should is too strong.  Change that to "it would be nice to", because the possibility that the base class destructor may be called (even if not by your code, possibly by others).

...

I find that it is being deleted when test_unit* is a stored in reference counted pointer,

Where?

My mistake, it's held in a map. See the next comment:

...

and at program exit it is being deleted by calling ~test_unit(). 

Where do I call destructor explicitly?

You don't.  It's being called implicitly at program exit, by valgrind.  I think what is happening is that valgrind replaces the delete operator with its own, which is "operator delete(void*)" The typeinfo is likely lost by this replacement of delete. Just because you don't call the destructor, it doesn't mean others won't.  If a base class is derived from, best practice is to declare its destructor virtual. It is relatively common to replace allocators (e.g. tcmalloc).

...

If you use a derived class (test_suite), this is leading to an error.

Where?

As already mentioned, reported by valgrind. 

...

Also, in the code I'm looking at (1.40.0), the test_suite class has a virtual destructor, but its base class  test_unit does not.  Not making the base class virtual seems to be defeating the purpose.

What purpose? The only reason that test_suite destructor is virtual is because this class in fact is being polymorphically. It has subclass master_test_suite.

By defeating the purpose, I mean it forces code like the following: if( ut_detail::test_id_2_unit_type( tu.second->p_id ) == tut_suite ) delete  static_cast(tu.second); else delete  static_cast(tu.second); A virtual base case simplifies this code (especially if in the future other classes are added, or in the case of replacement allocator it does the right thing). thanks, Scott

Gennadiy

_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users

__________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com