gdal icon indicating copy to clipboard operation
gdal copied to clipboard

[lint] OGRGeometry classes: add[Geometry/Curve/Ringetc.]Directly() methods are error prone

Open rouault opened this issue 4 years ago • 1 comments

The semantics of the add[Geometry/Curve/Ringetc.]Directly() methods in the OGRGeometry derived classes is error prone. When it succeeds, it acquires the passed geometry. But when it fails, it doesn't (thus it is the responsibility of the caller to free it). See https://github.com/OSGeo/gdal/pull/4370

We'd be much better with for example OGRCompoundCurve::addCurve(std::unique_ptr<OGRCurve>&& poCurve) that would take ownership.

Proposed plan:

  • rename the current methods with a leading underscore and make them private
  • make the current addXXXXDirectly() methods call that private method, and mark them deprecated
  • add the addXXXX(std::unique_ptr<YYY>&&) methods using the private method (and destroying the object when the internal method) fails
  • grep through the code base to remove our use of the deprecated methods (the remaining use would be the public C wrapper)

rouault avatar Aug 28 '21 15:08 rouault

Partially implemented per:

commit cf07c4ff648b43285fa00aa9ca3d78cfc32293b3 Author: Even Rouault [email protected] Date: Sat Sep 2 23:37:44 2023 +0200

OGRGeometry classes: add addGeometry()/addRing()/addCurve() methods accepting a std::unique_ptr

commit d04118ba47e0b02c12d587fbdd4a0e75b51e96f8 Author: Even Rouault [email protected] Date: Sat Sep 2 23:38:19 2023 +0200

gml2ogrgeometry: use std::unique_ptr

rouault avatar Apr 18 '24 17:04 rouault