Destructors of derived classes are automatically virtual if the base class destructor is virtual.
http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7 The inheritance chain is: LLMouseHandler > LLTool > LLManip > LLManipScale ... and both LLMouseHandler and LLTool declare the destructor as virtual. It wouldn't hurt to explicitly mark ~LLManip and ~LLManipScale as virtual for clarity in the code, but it won't actually change anything. On Fri, Jul 1, 2011 at 4:02 PM, Ricky <kf6...@gmail.com> wrote: > Looks like the destructors might only be called at program > termination, so this may not be a big problem anyway. However it IS > inconsistent and weird. And I have it within reach to clean up if it > seems good to do so. > > Ricky > Cron Stardust > > On Fri, Jul 1, 2011 at 3:25 PM, Ricky <kf6...@gmail.com> wrote: > > Poking around in the llmanip* files working on VWR-25739, I started to > > get annoyed at the coding inconsistencies between those files. So I > > started looking at what it would take to make the 3 subclasses > > (translate, scale, and rotate) consistent, when I tripped across the > > detail that llmaniptranslate.h has the destructor declared virtual > > while llmanipscale.h has it declared plainly, and llmaniprotate.h > > doesn't explicitly declare a destructor. > > > > When I looked up some reasons why a destructor should be virtual it > > seems that it should be virtual when the class is going to be used in > > a polymorphic way and will have delete called on a pointer to it. IE: > > // MyClass is a ParentClass > > ParentClass* p = new MyClass(); > > destroy p; > > > > Apparently this is about the only case for declaring the destructor > > virtual. (see > http://blogs.msdn.com/b/oldnewthing/archive/2004/05/07/127826.aspx > > and especially http://www.erata.net/programming/virtual-destructors/ ) > > It also comes with a minor performance hit, but that's outside of > > scope. > > > > It turns out that LLManipScale _is_ being used in such a way in > > LLToolComp - as are LLManipScale and LLManipRotate: > > lltoolcomp.h line 92: LLManip* mManip; > > lltoolcomp.cpp line 194: mManip = new LLManipTranslate(this); > > lltoolcomp.cpp line 203: delete mManip; > > lltoolcomp.cpp line 321: mManip = new LLManipScale(this); > > lltoolcomp.cpp line 330: delete mManip; > > lltoolcomp.cpp line 520: mManip = new LLManipRotate(this); > > lltoolcomp.cpp line 530: delete mManip; > > > > So it looks like to me that there might be a memory leak in the scale > > and rotate classes, as their destructors might NOT be being called. > > Of course, Translate's destructor has only an empty definition, and > > Rotate doesn't even have one, but Scale does have a full-on > > destructor. And because it is not virtual, it might not be being > > called. > > > > Looking over the history of the files gives me the following: > > The Rotate destructor was last touched by Steven Bennets on 2008-03-11 > > in rev 341 - when LLLinkedList was culled in favor of another > > technique. > > The Translate destructor was emptied by James Cook on 2009-12-10 in > > rev 4496 - switched to a std::vector > > The Scale destructor seems to have never existed in revision history. > > > > Anyone with more familiarity with C++'s nuances in such cases have any > > thoughts/suggestions? > > > > Ricky > > Cron Stardust > > > _______________________________________________ > Policies and (un)subscribe information available here: > http://wiki.secondlife.com/wiki/OpenSource-Dev > Please read the policies before posting to keep unmoderated posting > privileges >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
