widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #01006
Re: [Merge] lp:~csirkeee/widelands/memory-leaks-2 into lp:widelands
Thank you for looking into this, that's very much appreciated!
Some comments:
1. In logic/tribe.cc, the m_compatibility_wares: Are the explicit std::string casts really needed?
2. Good catch on the FileSystem handling. However, I would say that the underlying issue is not fixed yet: The interface is badly designed, it is very easy to shoot oneself in the foot with it. Returning a reference to a heap-allocated object has always been bad style, and it is particularly bad in modern C++.
I think an even better fix would be to change the interface of FileSystem in a consistent way so that those functions (Make/CreateSubFileSystem) return a boost::shared_ptr<FileSystem> instead of a reference. In that way, the problem is fixed automatically where FileSystems are used, instead of putting the burden on each call site to pay proper attention.
--
https://code.launchpad.net/~csirkeee/widelands/memory-leaks-2/+merge/150290
Your team Widelands Developers is requested to review the proposed merge of lp:~csirkeee/widelands/memory-leaks-2 into lp:widelands.
References