Skip to content

Commit

Permalink
Fix a LOT of leaks relating to geometry and GEOS operations
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Oct 21, 2015
1 parent a460e47 commit f5a72c6
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 30 deletions.
7 changes: 4 additions & 3 deletions src/core/geometry/qgsgeometry.h
Expand Up @@ -329,9 +329,10 @@ class CORE_EXPORT QgsGeometry
int addPart( QgsAbstractGeometryV2* part, QGis::GeometryType geomType = QGis::UnknownGeometry );

/** Adds a new island polygon to a multipolygon feature
@return 0 in case of success, 1 if not a multipolygon, 2 if ring is not a valid geometry, 3 if new polygon ring
not disjoint with existing polygons of the feature
@note not available in python bindings
* @param newPart part to add. Ownership is NOT transferred.
* @return 0 in case of success, 1 if not a multipolygon, 2 if ring is not a valid geometry, 3 if new polygon ring
* not disjoint with existing polygons of the feature
* @note not available in python bindings
*/
int addPart( GEOSGeometry *newPart );

Expand Down
63 changes: 36 additions & 27 deletions src/core/geometry/qgsgeos.cpp
Expand Up @@ -123,6 +123,10 @@ QgsGeos::QgsGeos( const QgsAbstractGeometryV2* geometry, double precision )

QgsGeos::~QgsGeos()
{
GEOSGeom_destroy_r( geosinit.ctxt, mGeos );
mGeos = 0;
GEOSPreparedGeom_destroy_r( geosinit.ctxt, mGeosPrepared );
mGeosPrepared = 0;
}

void QgsGeos::geometryChanged()
Expand Down Expand Up @@ -218,6 +222,8 @@ double QgsGeos::distance( const QgsAbstractGeometryV2& geom, QString* errorMsg )
}
CATCH_GEOS_WITH_ERRMSG( -1.0 )

GEOSGeom_destroy_r( geosinit.ctxt, otherGeosGeom );

return distance;
}

Expand Down Expand Up @@ -578,6 +584,7 @@ int QgsGeos::splitLinearGeometry( GEOSGeometry* splitLine, QList<QgsAbstractGeom
for ( int i = 0; i < lineGeoms.size(); ++i )
{
newGeometries << fromGeos( lineGeoms[i] );
GEOSGeom_destroy_r( geosinit.ctxt, lineGeoms[i] );
}

GEOSGeom_destroy_r( geosinit.ctxt, splitGeom );
Expand Down Expand Up @@ -644,6 +651,7 @@ int QgsGeos::splitPolygonGeometry( GEOSGeometry* splitLine, QList<QgsAbstractGeo

GEOSGeom_destroy_r( geosinit.ctxt, intersectGeometry );
}
GEOSGeom_destroy_r( geosinit.ctxt, polygons );

bool splitDone = true;
int nGeometriesThis = numberOfGeometries( mGeos ); //original number of geometries
Expand Down Expand Up @@ -679,7 +687,6 @@ int QgsGeos::splitPolygonGeometry( GEOSGeometry* splitLine, QList<QgsAbstractGeo
for ( i = 0; i < testedGeometries.size(); ++i )
newGeometries << fromGeos( testedGeometries[i] );

GEOSGeom_destroy_r( geosinit.ctxt, polygons );
return 0;
}

Expand Down Expand Up @@ -1213,13 +1220,13 @@ QgsAbstractGeometryV2* QgsGeos::buffer( double distance, int segments, QString*
return 0;
}

GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSBuffer_r( geosinit.ctxt, mGeos, distance, segments );
geos.reset( GEOSBuffer_r( geosinit.ctxt, mGeos, distance, segments ) );
}
CATCH_GEOS_WITH_ERRMSG( 0 );
return fromGeos( geos );
return fromGeos( geos.get() );
}

QgsAbstractGeometryV2 *QgsGeos::buffer( double distance, int segments, int endCapStyle, int joinStyle, double mitreLimit, QString* errorMsg ) const
Expand All @@ -1232,13 +1239,13 @@ QgsAbstractGeometryV2 *QgsGeos::buffer( double distance, int segments, int endCa
#if defined(GEOS_VERSION_MAJOR) && defined(GEOS_VERSION_MINOR) && \
((GEOS_VERSION_MAJOR>3) || ((GEOS_VERSION_MAJOR==3) && (GEOS_VERSION_MINOR>=3)))

GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSBufferWithStyle_r( geosinit.ctxt, mGeos, distance, segments, endCapStyle, joinStyle, mitreLimit );
geos.reset( GEOSBufferWithStyle_r( geosinit.ctxt, mGeos, distance, segments, endCapStyle, joinStyle, mitreLimit ) );
}
CATCH_GEOS_WITH_ERRMSG( 0 );
return fromGeos( geos );
return fromGeos( geos.get() );
#else
return 0;
#endif //0
Expand All @@ -1250,13 +1257,13 @@ QgsAbstractGeometryV2* QgsGeos::simplify( double tolerance, QString* errorMsg )
{
return 0;
}
GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSTopologyPreserveSimplify_r( geosinit.ctxt, mGeos, tolerance );
geos.reset( GEOSTopologyPreserveSimplify_r( geosinit.ctxt, mGeos, tolerance ) );
}
CATCH_GEOS_WITH_ERRMSG( 0 );
return fromGeos( geos );
return fromGeos( geos.get() );
}

QgsAbstractGeometryV2* QgsGeos::interpolate( double distance, QString* errorMsg ) const
Expand All @@ -1265,13 +1272,13 @@ QgsAbstractGeometryV2* QgsGeos::interpolate( double distance, QString* errorMsg
{
return 0;
}
GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSInterpolate_r( geosinit.ctxt, mGeos, distance );
geos.reset( GEOSInterpolate_r( geosinit.ctxt, mGeos, distance ) );
}
CATCH_GEOS_WITH_ERRMSG( 0 );
return fromGeos( geos );
return fromGeos( geos.get() );
}

bool QgsGeos::centroid( QgsPointV2& pt, QString* errorMsg ) const
Expand All @@ -1281,10 +1288,10 @@ bool QgsGeos::centroid( QgsPointV2& pt, QString* errorMsg ) const
return false;
}

GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSGetCentroid_r( geosinit.ctxt, mGeos );
geos.reset( GEOSGetCentroid_r( geosinit.ctxt, mGeos ) );
}
CATCH_GEOS_WITH_ERRMSG( false );

Expand All @@ -1294,8 +1301,8 @@ bool QgsGeos::centroid( QgsPointV2& pt, QString* errorMsg ) const
}

double x, y;
GEOSGeomGetX_r( geosinit.ctxt, geos, &x );
GEOSGeomGetY_r( geosinit.ctxt, geos, &y );
GEOSGeomGetX_r( geosinit.ctxt, geos.get(), &x );
GEOSGeomGetY_r( geosinit.ctxt, geos.get(), &y );
pt.setX( x ); pt.setY( y );
return true;
}
Expand All @@ -1306,13 +1313,13 @@ QgsAbstractGeometryV2* QgsGeos::envelope( QString* errorMsg ) const
{
return 0;
}
GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSEnvelope_r( geosinit.ctxt, mGeos );
geos.reset( GEOSEnvelope_r( geosinit.ctxt, mGeos ) );
}
CATCH_GEOS_WITH_ERRMSG( 0 );
return fromGeos( geos );
return fromGeos( geos.get() );
}

bool QgsGeos::pointOnSurface( QgsPointV2& pt, QString* errorMsg ) const
Expand All @@ -1322,10 +1329,10 @@ bool QgsGeos::pointOnSurface( QgsPointV2& pt, QString* errorMsg ) const
return false;
}

GEOSGeometry* geos = 0;
GEOSGeomScopedPtr geos;
try
{
geos = GEOSPointOnSurface_r( geosinit.ctxt, mGeos );
geos.reset( GEOSPointOnSurface_r( geosinit.ctxt, mGeos ) );
}
CATCH_GEOS_WITH_ERRMSG( false );

Expand All @@ -1335,8 +1342,8 @@ bool QgsGeos::pointOnSurface( QgsPointV2& pt, QString* errorMsg ) const
}

double x, y;
GEOSGeomGetX_r( geosinit.ctxt, geos, &x );
GEOSGeomGetY_r( geosinit.ctxt, geos, &y );
GEOSGeomGetX_r( geosinit.ctxt, geos.get(), &x );
GEOSGeomGetY_r( geosinit.ctxt, geos.get(), &y );

pt.setX( x );
pt.setY( y );
Expand Down Expand Up @@ -1613,6 +1620,7 @@ QgsAbstractGeometryV2* QgsGeos::reshapeGeometry( const QgsLineStringV2& reshapeW
if ( numGeoms == -1 )
{
if ( errorCode ) { *errorCode = 1; }
GEOSGeom_destroy_r( geosinit.ctxt, reshapeLineGeos );
return 0;
}

Expand All @@ -1638,6 +1646,7 @@ QgsAbstractGeometryV2* QgsGeos::reshapeGeometry( const QgsLineStringV2& reshapeW
if ( errorCode ) { *errorCode = 0; }
QgsAbstractGeometryV2* reshapeResult = fromGeos( reshapedGeometry );
GEOSGeom_destroy_r( geosinit.ctxt, reshapedGeometry );
GEOSGeom_destroy_r( geosinit.ctxt, reshapeLineGeos );
return reshapeResult;
}
else
Expand Down Expand Up @@ -2098,11 +2107,11 @@ int QgsGeos::pointContainedInLine( const GEOSGeometry* point, const GEOSGeometry

int QgsGeos::geomDigits( const GEOSGeometry* geom )
{
GEOSGeometry* bbox = GEOSEnvelope_r( geosinit.ctxt, geom );
if ( !bbox )
GEOSGeomScopedPtr bbox( GEOSEnvelope_r( geosinit.ctxt, geom ) );
if ( !bbox.get() )
return -1;

const GEOSGeometry* bBoxRing = GEOSGetExteriorRing_r( geosinit.ctxt, bbox );
const GEOSGeometry* bBoxRing = GEOSGetExteriorRing_r( geosinit.ctxt, bbox.get() );
if ( !bBoxRing )
return -1;

Expand Down
6 changes: 6 additions & 0 deletions src/core/geometry/qgsgeos.h
Expand Up @@ -84,6 +84,9 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
QgsAbstractGeometryV2* offsetCurve( double distance, int segments, int joinStyle, double mitreLimit, QString* errorMsg = 0 ) const override;
QgsAbstractGeometryV2* reshapeGeometry( const QgsLineStringV2& reshapeWithLine, int* errorCode, QString* errorMsg = 0 ) const;

/** Create a geometry from a GEOSGeometry
* @param goes GEOSGeometry. Ownership is NOT transferred.
*/
static QgsAbstractGeometryV2* fromGeos( const GEOSGeometry* geos );
static QgsPolygonV2* fromGeosPolygon( const GEOSGeometry* geos );
static GEOSGeometry* asGeos( const QgsAbstractGeometryV2* geom , double precision = 0 );
Expand Down Expand Up @@ -124,6 +127,9 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
static int numberOfGeometries( GEOSGeometry* g );
static GEOSGeometry* nodeGeometries( const GEOSGeometry *splitLine, const GEOSGeometry *geom );
int mergeGeometriesMultiTypeSplit( QVector<GEOSGeometry*>& splitResult ) const;

/** Ownership of geoms is transferred
*/
static GEOSGeometry* createGeosCollection( int typeId, const QVector<GEOSGeometry*>& geoms );

static GEOSGeometry* createGeosPoint( const QgsAbstractGeometryV2* point, int coordDims , double precision );
Expand Down

0 comments on commit f5a72c6

Please sign in to comment.