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).
This commit is contained in:
Nyall Dawson 2016-01-24 15:15:29 +11:00
parent 1dc8fc3c4e
commit 8ad6ca08fe
11 changed files with 35 additions and 34 deletions

View File

@ -345,9 +345,9 @@ class QgsExpressionContext
void appendScope( QgsExpressionContextScope* scope /Transfer/ );
/**
* Remove the last scope from the expression context.
* Removes the last scope from the expression context and return it.
*/
void popScope();
QgsExpressionContextScope* popScope();
/** Appends a scope to the end of the context. This scope will override
* any matching variables or functions provided by existing scopes within the
@ -491,14 +491,12 @@ class QgsExpressionContextUtils
static QgsExpressionContextScope* mapSettingsScope( const QgsMapSettings &mapSettings ) /Factory/;
/**
* Updates a symbol scope related to a QgsSymbolV2 to an expression context. If there is no existing scope
* provided, a new one will be generated and returned.
* Updates a symbol scope related to a QgsSymbolV2 to an expression context.
* @param symbol symbol to extract properties from
* @param symbolScope optional pointer to an existing scope to update
* @param symbolScope pointer to an existing scope to update
* @note added in QGIS 2.14
*/
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr ) /Factory/;
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr );
/** Creates a new scope which contains variables and functions relating to a QgsComposition.
* For instance, number of pages and page sizes.

View File

@ -50,7 +50,7 @@ static QgsExpressionContext _getExpressionContext( const void* context )
if ( layer )
expContext << QgsExpressionContextUtils::layerScope( layer );
expContext << QgsExpressionContextUtils::updateSymbolScope( nullptr );
expContext << QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
//TODO - show actual value
expContext.setOriginalValueVariable( QVariant() );

View File

@ -384,10 +384,12 @@ void QgsExpressionContext::appendScope( QgsExpressionContextScope* scope )
mStack.append( scope );
}
void QgsExpressionContext::popScope()
QgsExpressionContextScope* QgsExpressionContext::popScope()
{
if ( !mStack.isEmpty() )
mStack.pop_back();
return mStack.takeLast();
return nullptr;
}
QgsExpressionContext& QgsExpressionContext::operator<<( QgsExpressionContextScope* scope )
@ -721,7 +723,7 @@ QgsExpressionContextScope* QgsExpressionContextUtils::mapSettingsScope( const Qg
QgsExpressionContextScope* QgsExpressionContextUtils::updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope )
{
if ( !symbolScope )
symbolScope = new QgsExpressionContextScope( QObject::tr( "Symbol Scope" ) );
return nullptr;
symbolScope->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_SYMBOL_COLOR, symbol ? symbol->color() : QColor(), true ) );

View File

@ -380,9 +380,9 @@ class CORE_EXPORT QgsExpressionContext
void appendScope( QgsExpressionContextScope* scope );
/**
* Remove the last scope from the expression context.
* Removes the last scope from the expression context and return it.
*/
void popScope();
QgsExpressionContextScope* popScope();
/** Appends a scope to the end of the context. This scope will override
* any matching variables or functions provided by existing scopes within the
@ -523,15 +523,13 @@ class CORE_EXPORT QgsExpressionContextUtils
static QgsExpressionContextScope* mapSettingsScope( const QgsMapSettings &mapSettings );
/**
* Updates a symbol scope related to a QgsSymbolV2 to an expression context. If there is no existing scope
* provided, a new one will be generated and returned.
* Updates a symbol scope related to a QgsSymbolV2 to an expression context.
* @param symbol symbol to extract properties from
* @param symbolScope optional pointer to an existing scope to update
* @param symbolScope pointer to an existing scope to update
* @note added in QGIS 2.14
*/
static QgsExpressionContextScope* updateSymbolScope( const QgsSymbolV2* symbol, QgsExpressionContextScope* symbolScope = nullptr );
/** Creates a new scope which contains variables and functions relating to a QgsComposition.
* For instance, number of pages and page sizes.
* @param composition source composition

View File

@ -134,7 +134,7 @@ class QgsExpressionSorter
indexedFeatures.append( indexedFeature );
}
expressionContext->popScope();
delete expressionContext->popScope();
qSort( indexedFeatures.begin(), indexedFeatures.end(), *this );

View File

@ -281,7 +281,8 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
request.setSubsetOfAttributes( attrNames, mFields );
QgsFeatureIterator fit = mSource->getFeatures( request );
QgsExpressionContextScope* symbolScope = nullptr;
QgsExpressionContextScope* symbolScope = new QgsExpressionContextScope();
ctx.expressionContext().appendScope( symbolScope );
QgsFeature fet;
while ( fit.nextFeature( fet ) )
{
@ -297,8 +298,6 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
if ( !symbols.isEmpty() )
{
symbolScope = QgsExpressionContextUtils::updateSymbolScope( symbols.at( 0 ), symbolScope );
if ( !ctx.expressionContext().scopes().contains( symbolScope ) )
ctx.expressionContext().appendScope( symbolScope );
}
}
ctx.expressionContext().setFeature( fet );
@ -306,7 +305,7 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
}
if ( ctx.expressionContext().lastScope() == symbolScope )
ctx.expressionContext().popScope();
delete ctx.expressionContext().popScope();
if ( mRenderer )
mRenderer->stopRender( ctx );

View File

@ -286,7 +286,7 @@ void QgsVectorLayerRenderer::setGeometryCachePointer( QgsGeometryCache* cache )
void QgsVectorLayerRenderer::drawRendererV2( QgsFeatureIterator& fit )
{
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr );
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
mContext.expressionContext().appendScope( symbolScope );
QgsFeature fet;
@ -366,7 +366,7 @@ void QgsVectorLayerRenderer::drawRendererV2( QgsFeatureIterator& fit )
}
}
mContext.expressionContext().popScope();
delete mContext.expressionContext().popScope();
stopRendererV2( nullptr );
}
@ -384,7 +384,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
selRenderer->startRender( mContext, mFields );
}
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr );
QgsExpressionContextScope* symbolScope = QgsExpressionContextUtils::updateSymbolScope( nullptr, new QgsExpressionContextScope() );
mContext.expressionContext().appendScope( symbolScope );
// 1. fetch features
@ -398,7 +398,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
{
qDebug( "rendering stop!" );
stopRendererV2( selRenderer );
mContext.expressionContext().popScope();
delete mContext.expressionContext().popScope();
return;
}
@ -460,7 +460,7 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
}
}
mContext.expressionContext().popScope();
delete mContext.expressionContext().popScope();
// find out the order
QgsSymbolV2LevelOrder levels;

View File

@ -441,7 +441,7 @@ void QgsSymbolV2::startRender( QgsRenderContext& context, const QgsFields* field
QgsSymbolV2RenderContext symbolContext( context, outputUnit(), mAlpha, false, mRenderHints, nullptr, fields, mapUnitScale() );
QgsExpressionContextScope* scope = QgsExpressionContextUtils::updateSymbolScope( this );
QgsExpressionContextScope* scope = QgsExpressionContextUtils::updateSymbolScope( this, new QgsExpressionContextScope() );
mSymbolRenderContext->setExpressionContextScope( scope );
@ -713,10 +713,13 @@ void QgsSymbolV2::renderFeature( const QgsFeature& feature, QgsRenderContext& co
deleteSegmentizedGeometry = true;
}
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_count", segmentizedGeometry->geometry()->partCount() );
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_num", 1 );
if ( mSymbolRenderContext->expressionContextScope() )
{
context.expressionContext().appendScope( mSymbolRenderContext->expressionContextScope() );
QgsExpressionContextUtils::updateSymbolScope( this, mSymbolRenderContext->expressionContextScope() );
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_count", segmentizedGeometry->geometry()->partCount() );
mSymbolRenderContext->expressionContextScope()->setVariable( "geometry_part_num", 1 );
}
switch ( QgsWKBTypes::flatType( segmentizedGeometry->geometry()->wkbType() ) )
{

View File

@ -48,6 +48,7 @@ Qgs25DRendererWidget::Qgs25DRendererWidget( QgsVectorLayer* layer, QgsStyleV2* s
QgsExpressionContextScope* scope = QgsExpressionContextUtils::layerScope( mLayer );
QVariant height = scope->variable( "qgis_25d_height" );
QVariant angle = scope->variable( "qgis_25d_angle" );
delete scope;
mHeightWidget->setField( height.isNull() ? "10" : height.toString() );
mAngleWidget->setValue( angle.isNull() ? 70 : angle.toDouble() );

View File

@ -523,9 +523,9 @@ int vtableBestIndex( sqlite3_vtab *pvtab, sqlite3_index_info* indexInfo )
break;
#ifdef SQLITE_INDEX_CONSTRAINT_LIKE
case SQLITE_INDEX_CONSTRAINT_LIKE:
#endif
expr += " LIKE ";
break;
#endif
default:
break;
}

View File

@ -295,7 +295,7 @@ void TestQgsExpressionContext::contextStack()
QVERIFY( !context.isReadOnly( "test" ) );
// Check scopes can be popped
context.popScope();
delete context.popScope();
QCOMPARE( scopes.length(), 2 );
QCOMPARE( scopes.at( 0 ), scope1 );
}