← Back to team overview

widelands-dev team mailing list archive

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