Skip to content

Commit 8ad6ca0

Browse files
nyalldawsonnyalldawson
committedJan 24, 2016
Fix some memory leaks identified by Coverity
Also improve the API for QgsExpressionContextUtils::updateSymbolScope Previously it was hard to predict if the method would create storage, which caused issues for the python bindings (eg, they had a choice of either leaking or being crashy).
·
ltr-3_40archive/master_2
1 parent 1dc8fc3 commit 8ad6ca0

11 files changed

+35
-34
lines changed
 

‎python/core/qgsexpressioncontext.sip

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,9 @@ class QgsExpressionContext
345345
void appendScope( QgsExpressionContextScope* scope /Transfer/ );
346346

347347
/**
348-
* Remove the last scope from the expression context.
348+
* Removes the last scope from the expression context and return it.
349349
*/
350-
void popScope();
350+
QgsExpressionContextScope* popScope();
351351

352352
/** Appends a scope to the end of the context. This scope will override
353353
* any matching variables or functions provided by existing scopes within the
@@ -491,14 +491,12 @@ class QgsExpressionContextUtils
491491
static QgsExpressionContextScope* mapSettingsScope( const QgsMapSettings &mapSettings ) /Factory/;
492492

493493
/**
494-
* Updates a symbol scope related to a QgsSymbolV2 to an expression context. If there is no existing scope
495-
* provided, a new one will be generated and returned.
494+
* Updates a symbol scope related to a QgsSymbolV2 to an expression context.
496495
* @param symbol symbol to extract properties from
497-
* @param symbolScope optional pointer to an existing scope to update
496+
* @param symbolScope pointer to an existing scope to update
498497
* @note added in QGIS 2.14
499498
*/
500-
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr ) /Factory/;
501-
499+
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr );
502500

503501
/** Creates a new scope which contains variables and functions relating to a QgsComposition.
504502
* For instance, number of pages and page sizes.

‎src/app/qgslabelinggui.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ static QgsExpressionContext _getExpressionContext( const void* context )
5050
if ( layer )
5151
expContext << QgsExpressionContextUtils::layerScope( layer );
5252

53-
expContext << QgsExpressionContextUtils::updateSymbolScope( nullptr );
53+
expContext << QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
5454

5555
//TODO - show actual value
5656
expContext.setOriginalValueVariable( QVariant() );

‎src/core/qgsexpressioncontext.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,12 @@ void QgsExpressionContext::appendScope( QgsExpressionContextScope* scope )
384384
mStack.append( scope );
385385
}
386386

387-
void QgsExpressionContext::popScope()
387+
QgsExpressionContextScope* QgsExpressionContext::popScope()
388388
{
389389
if ( !mStack.isEmpty() )
390-
mStack.pop_back();
390+
return mStack.takeLast();
391+
392+
return nullptr;
391393
}
392394

393395
QgsExpressionContext& QgsExpressionContext::operator<<( QgsExpressionContextScope* scope )
@@ -721,7 +723,7 @@ QgsExpressionContextScope* QgsExpressionContextUtils::mapSettingsScope( const Qg
721723
QgsExpressionContextScope* QgsExpressionContextUtils::updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope )
722724
{
723725
if ( !symbolScope )
724-
symbolScope = new QgsExpressionContextScope( QObject::tr( "Symbol Scope" ) );
726+
return nullptr;
725727

726728
symbolScope->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_SYMBOL_COLOR, symbol ? symbol->color() : QColor(), true ) );
727729

‎src/core/qgsexpressioncontext.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,9 @@ class CORE_EXPORT QgsExpressionContext
380380
void appendScope( QgsExpressionContextScope* scope );
381381

382382
/**
383-
* Remove the last scope from the expression context.
383+
* Removes the last scope from the expression context and return it.
384384
*/
385-
void popScope();
385+
QgsExpressionContextScope* popScope();
386386

387387
/** Appends a scope to the end of the context. This scope will override
388388
* any matching variables or functions provided by existing scopes within the
@@ -523,15 +523,13 @@ class CORE_EXPORT QgsExpressionContextUtils
523523
static QgsExpressionContextScope* mapSettingsScope( const QgsMapSettings &mapSettings );
524524

525525
/**
526-
* Updates a symbol scope related to a QgsSymbolV2 to an expression context. If there is no existing scope
527-
* provided, a new one will be generated and returned.
526+
* Updates a symbol scope related to a QgsSymbolV2 to an expression context.
528527
* @param symbol symbol to extract properties from
529-
* @param symbolScope optional pointer to an existing scope to update
528+
* @param symbolScope pointer to an existing scope to update
530529
* @note added in QGIS 2.14
531530
*/
532531
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr );
533532

534-
535533
/** Creates a new scope which contains variables and functions relating to a QgsComposition.
536534
* For instance, number of pages and page sizes.
537535
* @param composition source composition

‎src/core/qgsexpressionsorter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class QgsExpressionSorter
134134
indexedFeatures.append( indexedFeature );
135135
}
136136

137-
expressionContext->popScope();
137+
delete expressionContext->popScope();
138138

139139
qSort( indexedFeatures.begin(), indexedFeatures.end(), *this );
140140

‎src/core/qgsvectorlayerlabelprovider.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
281281
request.setSubsetOfAttributes( attrNames, mFields );
282282
QgsFeatureIterator fit = mSource->getFeatures( request );
283283

284-
QgsExpressionContextScope* symbolScope = nullptr;
284+
QgsExpressionContextScope* symbolScope = new QgsExpressionContextScope();
285+
ctx.expressionContext().appendScope( symbolScope );
285286
QgsFeature fet;
286287
while ( fit.nextFeature( fet ) )
287288
{
@@ -297,16 +298,14 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
297298
if ( !symbols.isEmpty() )
298299
{
299300
symbolScope = QgsExpressionContextUtils::updateSymbolScope( symbols.at( 0 ), symbolScope );
300-
if ( !ctx.expressionContext().scopes().contains( symbolScope ) )
301-
ctx.expressionContext().appendScope( symbolScope );
302301
}
303302
}
304303
ctx.expressionContext().setFeature( fet );
305304
registerFeature( fet, ctx, obstacleGeometry.data() );
306305
}
307306

308307
if ( ctx.expressionContext().lastScope() == symbolScope )
309-
ctx.expressionContext().popScope();
308+
delete ctx.expressionContext().popScope();
310309

311310
if ( mRenderer )
312311
mRenderer->stopRender( ctx );

‎src/core/qgsvectorlayerrenderer.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ void QgsVectorLayerRenderer::setGeometryCachePointer( QgsGeometryCache* cache )
286286

287287
void QgsVectorLayerRenderer::drawRendererV2( QgsFeatureIterator& fit )
288288
{
289-
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr );
289+
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
290290
mContext.expressionContext().appendScope( symbolScope );
291291

292292
QgsFeature fet;
@@ -366,7 +366,7 @@ void QgsVectorLayerRenderer::drawRendererV2( QgsFeatureIterator& fit )
366366
}
367367
}
368368

369-
mContext.expressionContext().popScope();
369+
delete mContext.expressionContext().popScope();
370370

371371
stopRendererV2( nullptr );
372372
}
@@ -384,7 +384,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
384384
selRenderer->startRender( mContext, mFields );
385385
}
386386

387-
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr );
387+
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
388388
mContext.expressionContext().appendScope( symbolScope );
389389

390390
// 1. fetch features
@@ -398,7 +398,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
398398
{
399399
qDebug( "rendering stop!" );
400400
stopRendererV2( selRenderer );
401-
mContext.expressionContext().popScope();
401+
delete mContext.expressionContext().popScope();
402402
return;
403403
}
404404

@@ -460,7 +460,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
460460
}
461461
}
462462

463-
mContext.expressionContext().popScope();
463+
delete mContext.expressionContext().popScope();
464464

465465
// find out the order
466466
QgsSymbolV2LevelOrder levels;

‎src/core/symbology-ng/qgssymbolv2.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ void QgsSymbolV2::startRender( QgsRenderContext& context, const QgsFields* field
441441

442442
QgsSymbolV2RenderContext symbolContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() );
443443

444-
QgsExpressionContextScope* scope = QgsExpressionContextUtils::updateSymbolScope( this );
444+
QgsExpressionContextScope* scope = QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() );
445445

446446
mSymbolRenderContext->setExpressionContextScope( scope );
447447

@@ -713,10 +713,13 @@ void QgsSymbolV2::renderFeature( const QgsFeature& feature, QgsRenderContext& co
713713
deleteSegmentizedGeometry = true;
714714
}
715715

716-
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
717-
QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
718-
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_count", segmentizedGeometry->geometry()->partCount() );
719-
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_num", 1 );
716+
if ( mSymbolRenderContext->expressionContextScope() )
717+
{
718+
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
719+
QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
720+
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_count", segmentizedGeometry->geometry()->partCount() );
721+
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_num", 1 );
722+
}
720723

721724
switch ( QgsWKBTypes::flatType( segmentizedGeometry->geometry()->wkbType() ) )
722725
{

‎src/gui/symbology-ng/qgs25drendererwidget.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Qgs25DRendererWidget::Qgs25DRendererWidget( QgsVectorLayer* layer, QgsStyleV2* s
4848
QgsExpressionContextScope* scope = QgsExpressionContextUtils::layerScope( mLayer );
4949
QVariant height = scope->variable( "qgis_25d_height" );
5050
QVariant angle = scope->variable( "qgis_25d_angle" );
51+
delete scope;
5152

5253
mHeightWidget->setField( height.isNull() ? "10" : height.toString() );
5354
mAngleWidget->setValue( angle.isNull() ? 70 : angle.toDouble() );

‎src/providers/virtual/qgsvirtuallayersqlitemodule.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,9 @@ int vtableBestIndex( sqlite3_vtab *pvtab, sqlite3_index_info* indexInfo )
523523
break;
524524
#ifdef SQLITE_INDEX_CONSTRAINT_LIKE
525525
case SQLITE_INDEX_CONSTRAINT_LIKE:
526-
#endif
527526
expr += " LIKE ";
528527
break;
528+
#endif
529529
default:
530530
break;
531531
}

‎tests/src/core/testqgsexpressioncontext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ void TestQgsExpressionContext::contextStack()
295295
QVERIFY( !context.isReadOnly( "test" ) );
296296

297297
// Check scopes can be popped
298-
context.popScope();
298+
delete context.popScope();
299299
QCOMPARE( scopes.length(), 2 );
300300
QCOMPARE( scopes.at( 0 ), scope1 );
301301
}

0 commit comments

Comments
 (0)