[geos-devel] Reviewing std::auto_ptr spaghetti
mateusz at loskot.net
Wed Sep 6 09:03:32 PDT 2017
On 6 September 2017 at 17:17, Sandro Santilli <strk at kbt.io> wrote:
> On Wed, Sep 06, 2017 at 04:24:45PM +0200, Mateusz Loskot wrote:
>> In the code, there are uses of std::auto_ptr that are hard to reason about
>> what makes me suspicious:
>> class T
>> auto_ptr items;
>> auto_ptr<A> getItems()
>> return items;
>> Is the intention to give up the object's internal resource, really?
> Yes. I agree it's a weird pattern, but it's what got us to the current
> leak-free state.
Yes, that was better than nothing.
> Very dangerous for C++ interface, but safe for C-API.
I'm not sure I understand the difference, unless you assume
a) 100% LOC coverage with tests and
b) GEOS never misuses such classes/interfaces as pointed above
> Best we can do is document when needed.
Yes, but since it seems an enormous code review task,
I'm hardly optimistic it's feasible.
Practically possible migration away from auto_ptr is:
2. Add std::move(ptr) wherever required.
3. Ensure GEOS compiles.
4. Ensure GEOS tests pass.
5. Document C++ API breakage.
6. Assume GEOS remains unbroken.
If I hear no objections, I will (try to) proceed as outlined.
Mateusz Loskot, http://mateusz.loskot.net
More information about the geos-devel