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
