widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Disapprove

Unfortunately, these fixes are wrong. cpp is wrong about there being a memory leak, but it hardly is to blame. 


Back in the days, we decided it was a good idea to let Panels manage the memory of their children (not a terrible idea, but it leads to confusing code now). In the constructor of Panel::Panel, the parent gets the child added to its list of children. In Panel::~Panel we call free_children() which deletes all children. Having the children non-pointers (like you did in this change) means that the delete calls will delete unmanaged memory - that is undefined behavior and could lead to crashes.

So the proper fix would be to change panel to actually hold std::unique_ptr<> to the children so that the ownership is clear. But that is a bit more involved to change. 
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck_memleak/+merge/300979
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck_memleak.