Skip to content

Commit

Permalink
Even more memory safety in geometry internals by using qgis::make_unique
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Oct 13, 2017
1 parent 85321b4 commit 8284575
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 70 deletions.
52 changes: 21 additions & 31 deletions src/core/geometry/qgsgeometry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,8 @@ double QgsGeometry::closestSegmentWithContext(

QgsGeometry::OperationResult QgsGeometry::addRing( const QList<QgsPointXY> &ring )
{
QgsLineString *ringLine = new QgsLineString( ring );
return addRing( ringLine );
std::unique_ptr< QgsLineString > ringLine = qgis::make_unique< QgsLineString >( ring );
return addRing( ringLine.release() );
}

QgsGeometry::OperationResult QgsGeometry::addRing( QgsCurve *ring )
Expand Down Expand Up @@ -654,11 +654,11 @@ QgsGeometry::OperationResult QgsGeometry::addPart( const QgsPointSequence &point
std::unique_ptr< QgsAbstractGeometry > partGeom;
if ( points.size() == 1 )
{
partGeom.reset( new QgsPoint( points[0] ) );
partGeom = qgis::make_unique< QgsPoint >( points[0] );
}
else if ( points.size() > 1 )
{
std::unique_ptr< QgsLineString > ringLine( new QgsLineString() );
std::unique_ptr< QgsLineString > ringLine = qgis::make_unique< QgsLineString >();
ringLine->setPoints( points );
partGeom = std::move( ringLine );
}
Expand All @@ -673,13 +673,13 @@ QgsGeometry::OperationResult QgsGeometry::addPart( QgsAbstractGeometry *part, Qg
switch ( geomType )
{
case QgsWkbTypes::PointGeometry:
detachAndReset( std::unique_ptr< QgsAbstractGeometry >( new QgsMultiPointV2() ) );
detachAndReset( qgis::make_unique< QgsMultiPointV2 >() );
break;
case QgsWkbTypes::LineGeometry:
detachAndReset( std::unique_ptr< QgsAbstractGeometry >( new QgsMultiLineString() ) );
detachAndReset( qgis::make_unique< QgsMultiLineString >() );
break;
case QgsWkbTypes::PolygonGeometry:
detachAndReset( std::unique_ptr< QgsAbstractGeometry >( new QgsMultiPolygonV2() ) );
detachAndReset( qgis::make_unique< QgsMultiPolygonV2 >() );
break;
default:
detachAndReset( nullptr );
Expand Down Expand Up @@ -1307,6 +1307,7 @@ QgsPolyline QgsGeometry::asPolyline() const

bool doSegmentation = ( QgsWkbTypes::flatType( d->geometry->wkbType() ) == QgsWkbTypes::CompoundCurve
|| QgsWkbTypes::flatType( d->geometry->wkbType() ) == QgsWkbTypes::CircularString );
std::unique_ptr< QgsLineString > segmentizedLine;
QgsLineString *line = nullptr;
if ( doSegmentation )
{
Expand All @@ -1315,7 +1316,8 @@ QgsPolyline QgsGeometry::asPolyline() const
{
return polyLine;
}
line = curve->curveToLine();
segmentizedLine.reset( curve->curveToLine() );
line = segmentizedLine.get();
}
else
{
Expand All @@ -1334,11 +1336,6 @@ QgsPolyline QgsGeometry::asPolyline() const
polyLine[i].setY( line->yAt( i ) );
}

if ( doSegmentation )
{
delete line;
}

return polyLine;
}

Expand All @@ -1350,14 +1347,16 @@ QgsPolygon QgsGeometry::asPolygon() const
bool doSegmentation = ( QgsWkbTypes::flatType( d->geometry->wkbType() ) == QgsWkbTypes::CurvePolygon );

QgsPolygonV2 *p = nullptr;
std::unique_ptr< QgsPolygonV2 > segmentized;
if ( doSegmentation )
{
QgsCurvePolygon *curvePoly = qgsgeometry_cast<QgsCurvePolygon *>( d->geometry.get() );
if ( !curvePoly )
{
return QgsPolygon();
}
p = curvePoly->toPolygon();
segmentized.reset( curvePoly->toPolygon() );
p = segmentized.get();
}
else
{
Expand All @@ -1372,10 +1371,6 @@ QgsPolygon QgsGeometry::asPolygon() const
QgsPolygon polygon;
convertPolygon( *p, polygon );

if ( doSegmentation )
{
delete p;
}
return polygon;
}

Expand Down Expand Up @@ -1425,29 +1420,24 @@ QgsMultiPolyline QgsGeometry::asMultiPolyline() const
QgsMultiPolyline mpl;
for ( int i = 0; i < nLines; ++i )
{
bool deleteLine = false;
const QgsLineString *line = qgsgeometry_cast<const QgsLineString *>( geomCollection->geometryN( i ) );
std::unique_ptr< QgsLineString > segmentized;
if ( !line )
{
const QgsCurve *curve = qgsgeometry_cast<const QgsCurve *>( geomCollection->geometryN( i ) );
if ( !curve )
{
continue;
}
deleteLine = true;
line = curve->curveToLine();
segmentized.reset( curve->curveToLine() );
line = segmentized.get();
}

QgsPointSequence lineCoords;
line->points( lineCoords );
QgsPolyline polyLine;
convertToPolyline( lineCoords, polyLine );
mpl.append( polyLine );

if ( deleteLine )
{
delete line;
}
}
return mpl;
}
Expand Down Expand Up @@ -2564,7 +2554,7 @@ GEOSContextHandle_t QgsGeometry::getGEOSHandler()

QgsGeometry QgsGeometry::fromQPointF( QPointF point )
{
return QgsGeometry( new QgsPoint( point.x(), point.y() ) );
return QgsGeometry( qgis::make_unique< QgsPoint >( point.x(), point.y() ) );
}

QgsGeometry QgsGeometry::fromQPolygonF( const QPolygonF &polygon )
Expand Down Expand Up @@ -2662,7 +2652,7 @@ QgsGeometry QgsGeometry::smooth( const unsigned int iterations, const double off
{
QgsMultiLineString *multiLine = static_cast< QgsMultiLineString * >( d->geometry.get() );

std::unique_ptr< QgsMultiLineString > resultMultiline( new QgsMultiLineString() );
std::unique_ptr< QgsMultiLineString > resultMultiline = qgis::make_unique< QgsMultiLineString> ();
for ( int i = 0; i < multiLine->numGeometries(); ++i )
{
resultMultiline->addGeometry( smoothLine( *( static_cast< QgsLineString * >( multiLine->geometryN( i ) ) ), iterations, offset, minimumDistance, maxAngle ).release() );
Expand All @@ -2680,7 +2670,7 @@ QgsGeometry QgsGeometry::smooth( const unsigned int iterations, const double off
{
QgsMultiPolygonV2 *multiPoly = static_cast< QgsMultiPolygonV2 * >( d->geometry.get() );

std::unique_ptr< QgsMultiPolygonV2 > resultMultiPoly( new QgsMultiPolygonV2() );
std::unique_ptr< QgsMultiPolygonV2 > resultMultiPoly = qgis::make_unique< QgsMultiPolygonV2 >();
for ( int i = 0; i < multiPoly->numGeometries(); ++i )
{
resultMultiPoly->addGeometry( smoothPolygon( *( static_cast< QgsPolygonV2 * >( multiPoly->geometryN( i ) ) ), iterations, offset, minimumDistance, maxAngle ).release() );
Expand All @@ -2705,7 +2695,7 @@ std::unique_ptr< QgsLineString > smoothCurve( const QgsLineString &line, const u
const double offset, double squareDistThreshold, double maxAngleRads,
bool isRing )
{
std::unique_ptr< QgsLineString > result( new QgsLineString( line ) );
std::unique_ptr< QgsLineString > result = qgis::make_unique< QgsLineString >( line );
for ( unsigned int iteration = 0; iteration < iterations; ++iteration )
{
QgsPointSequence outputLine;
Expand Down Expand Up @@ -2797,7 +2787,7 @@ std::unique_ptr<QgsPolygonV2> QgsGeometry::smoothPolygon( const QgsPolygonV2 &po
{
double maxAngleRads = maxAngle * M_PI / 180.0;
double squareDistThreshold = minimumDistance > 0 ? minimumDistance * minimumDistance : -1;
std::unique_ptr< QgsPolygonV2 > resultPoly( new QgsPolygonV2 );
std::unique_ptr< QgsPolygonV2 > resultPoly = qgis::make_unique< QgsPolygonV2 >();

resultPoly->setExteriorRing( smoothCurve( *( static_cast< const QgsLineString *>( polygon.exteriorRing() ) ), iterations, offset,
squareDistThreshold, maxAngleRads, true ).release() );
Expand Down
4 changes: 2 additions & 2 deletions src/core/geometry/qgsgeometryeditutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,11 @@ QgsGeometry::OperationResult QgsGeometryEditUtils::addPart( QgsAbstractGeometry
std::unique_ptr<QgsCurvePolygon> poly;
if ( QgsWkbTypes::flatType( curve->wkbType() ) == QgsWkbTypes::LineString )
{
poly.reset( new QgsPolygonV2() );
poly = qgis::make_unique< QgsPolygonV2 >();
}
else
{
poly.reset( new QgsCurvePolygon() );
poly = qgis::make_unique< QgsCurvePolygon >();
}
// Ownership is still with part, curve points to the same object and is transferred
// to poly here.
Expand Down
74 changes: 37 additions & 37 deletions src/core/geometry/qgsgeometryfactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,51 +75,51 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryFactory::geomFromWkt( const QStr
std::unique_ptr< QgsAbstractGeometry> geom;
if ( trimmed.startsWith( QLatin1String( "Point" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsPoint() );
geom = qgis::make_unique< QgsPoint >();
}
else if ( trimmed.startsWith( QLatin1String( "LineString" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsLineString() );
geom = qgis::make_unique< QgsLineString >();
}
else if ( trimmed.startsWith( QLatin1String( "CircularString" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsCircularString() );
geom = qgis::make_unique< QgsCircularString >();
}
else if ( trimmed.startsWith( QLatin1String( "CompoundCurve" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsCompoundCurve() );
geom = qgis::make_unique< QgsCompoundCurve>();
}
else if ( trimmed.startsWith( QLatin1String( "Polygon" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsPolygonV2() );
geom = qgis::make_unique< QgsPolygonV2 >();
}
else if ( trimmed.startsWith( QLatin1String( "CurvePolygon" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsCurvePolygon() );
geom = qgis::make_unique< QgsCurvePolygon >();
}
else if ( trimmed.startsWith( QLatin1String( "MultiPoint" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsMultiPointV2() );
geom = qgis::make_unique< QgsMultiPointV2 >();
}
else if ( trimmed.startsWith( QLatin1String( "MultiCurve" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsMultiCurve() );
geom = qgis::make_unique< QgsMultiCurve >();
}
else if ( trimmed.startsWith( QLatin1String( "MultiLineString" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsMultiLineString() );
geom = qgis::make_unique< QgsMultiLineString >();
}
else if ( trimmed.startsWith( QLatin1String( "MultiSurface" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsMultiSurface() );
geom = qgis::make_unique< QgsMultiSurface >();
}
else if ( trimmed.startsWith( QLatin1String( "MultiPolygon" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsMultiPolygonV2() );
geom = qgis::make_unique< QgsMultiPolygonV2 >();
}
else if ( trimmed.startsWith( QLatin1String( "GeometryCollection" ), Qt::CaseInsensitive ) )
{
geom.reset( new QgsGeometryCollection() );
geom = qgis::make_unique< QgsGeometryCollection >();
}

if ( geom )
Expand All @@ -134,12 +134,12 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryFactory::geomFromWkt( const QStr

std::unique_ptr< QgsAbstractGeometry > QgsGeometryFactory::fromPoint( const QgsPointXY &point )
{
return std::unique_ptr< QgsAbstractGeometry >( new QgsPoint( point.x(), point.y() ) );
return qgis::make_unique< QgsPoint >( point.x(), point.y() );
}

std::unique_ptr<QgsMultiPointV2> QgsGeometryFactory::fromMultiPoint( const QgsMultiPoint &multipoint )
{
std::unique_ptr< QgsMultiPointV2 > mp( new QgsMultiPointV2() );
std::unique_ptr< QgsMultiPointV2 > mp = qgis::make_unique< QgsMultiPointV2 >();
QgsMultiPoint::const_iterator ptIt = multipoint.constBegin();
for ( ; ptIt != multipoint.constEnd(); ++ptIt )
{
Expand All @@ -156,7 +156,7 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryFactory::fromPolyline( const Qgs

std::unique_ptr<QgsMultiLineString> QgsGeometryFactory::fromMultiPolyline( const QgsMultiPolyline &multiline )
{
std::unique_ptr< QgsMultiLineString > mLine( new QgsMultiLineString() );
std::unique_ptr< QgsMultiLineString > mLine = qgis::make_unique< QgsMultiLineString >();
for ( int i = 0; i < multiline.size(); ++i )
{
mLine->addGeometry( fromPolyline( multiline.at( i ) ).release() );
Expand All @@ -166,7 +166,7 @@ std::unique_ptr<QgsMultiLineString> QgsGeometryFactory::fromMultiPolyline( const

std::unique_ptr<QgsPolygonV2> QgsGeometryFactory::fromPolygon( const QgsPolygon &polygon )
{
std::unique_ptr< QgsPolygonV2 > poly( new QgsPolygonV2() );
std::unique_ptr< QgsPolygonV2 > poly = qgis::make_unique< QgsPolygonV2 >();

QList<QgsCurve *> holes;
for ( int i = 0; i < polygon.size(); ++i )
Expand All @@ -189,7 +189,7 @@ std::unique_ptr<QgsPolygonV2> QgsGeometryFactory::fromPolygon( const QgsPolygon

std::unique_ptr< QgsMultiPolygonV2 > QgsGeometryFactory::fromMultiPolygon( const QgsMultiPolygon &multipoly )
{
std::unique_ptr< QgsMultiPolygonV2 > mp( new QgsMultiPolygonV2() );
std::unique_ptr< QgsMultiPolygonV2 > mp = qgis::make_unique< QgsMultiPolygonV2 >();
for ( int i = 0; i < multipoly.size(); ++i )
{
mp->addGeometry( fromPolygon( multipoly.at( i ) ).release() );
Expand All @@ -209,7 +209,7 @@ std::unique_ptr<QgsLineString> QgsGeometryFactory::linestringFromPolyline( const
x << it->x();
y << it->y();
}
std::unique_ptr< QgsLineString > line( new QgsLineString( x, y ) );
std::unique_ptr< QgsLineString > line = qgis::make_unique< QgsLineString >( x, y );
return line;
}

Expand All @@ -219,31 +219,31 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryFactory::geomFromWkbType( QgsWkb
switch ( type )
{
case QgsWkbTypes::Point:
return std::unique_ptr<QgsAbstractGeometry>( new QgsPoint() );
return qgis::make_unique< QgsPoint >();
case QgsWkbTypes::LineString:
return std::unique_ptr<QgsAbstractGeometry>( new QgsLineString() );
return qgis::make_unique< QgsLineString >();
case QgsWkbTypes::CircularString:
return std::unique_ptr<QgsAbstractGeometry>( new QgsCircularString() );
return qgis::make_unique< QgsCircularString >();
case QgsWkbTypes::CompoundCurve:
return std::unique_ptr<QgsAbstractGeometry>( new QgsCompoundCurve() );
return qgis::make_unique< QgsCompoundCurve >();
case QgsWkbTypes::Polygon:
return std::unique_ptr<QgsAbstractGeometry>( new QgsPolygonV2() );
return qgis::make_unique< QgsPolygonV2 >();
case QgsWkbTypes::CurvePolygon:
return std::unique_ptr<QgsAbstractGeometry>( new QgsCurvePolygon() );
return qgis::make_unique< QgsCurvePolygon >();
case QgsWkbTypes::MultiLineString:
return std::unique_ptr<QgsAbstractGeometry>( new QgsMultiLineString() );
return qgis::make_unique< QgsMultiLineString >();
case QgsWkbTypes::MultiPolygon:
return std::unique_ptr<QgsAbstractGeometry>( new QgsMultiPolygonV2() );
return qgis::make_unique< QgsMultiPolygonV2 >();
case QgsWkbTypes::MultiPoint:
return std::unique_ptr<QgsAbstractGeometry>( new QgsMultiPointV2() );
return qgis::make_unique< QgsMultiPointV2 >();
case QgsWkbTypes::MultiCurve:
return std::unique_ptr<QgsAbstractGeometry>( new QgsMultiCurve() );
return qgis::make_unique< QgsMultiCurve >();
case QgsWkbTypes::MultiSurface:
return std::unique_ptr<QgsAbstractGeometry>( new QgsMultiSurface() );
return qgis::make_unique< QgsMultiSurface >();
case QgsWkbTypes::GeometryCollection:
return std::unique_ptr<QgsAbstractGeometry>( new QgsGeometryCollection() );
return qgis::make_unique< QgsGeometryCollection >();
case QgsWkbTypes::Triangle:
return std::unique_ptr<QgsAbstractGeometry>( new QgsTriangle() );
return qgis::make_unique< QgsTriangle >();
default:
return nullptr;
}
Expand All @@ -256,22 +256,22 @@ std::unique_ptr<QgsGeometryCollection> QgsGeometryFactory::createCollectionOfTyp
switch ( type )
{
case QgsWkbTypes::MultiPoint:
collect.reset( new QgsMultiPointV2() );
collect = qgis::make_unique< QgsMultiPointV2 >();
break;
case QgsWkbTypes::MultiLineString:
collect.reset( new QgsMultiLineString() );
collect = qgis::make_unique< QgsMultiLineString >();
break;
case QgsWkbTypes::MultiCurve:
collect.reset( new QgsMultiCurve() );
collect = qgis::make_unique< QgsMultiCurve >();
break;
case QgsWkbTypes::MultiPolygon:
collect.reset( new QgsMultiPolygonV2() );
collect = qgis::make_unique< QgsMultiPolygonV2 >();
break;
case QgsWkbTypes::MultiSurface:
collect.reset( new QgsMultiSurface() );
collect = qgis::make_unique< QgsMultiSurface >();
break;
case QgsWkbTypes::GeometryCollection:
collect.reset( new QgsGeometryCollection() );
collect = qgis::make_unique< QgsGeometryCollection >();
break;
default:
// should not be possible
Expand Down

0 comments on commit 8284575

Please sign in to comment.