Skip to content

Commit 850e70b

Browse files
nyalldawsonnyalldawson
committedJul 19, 2018
Fix potential crash in symbol list widget
Possible fix for a crash obtained with incomplete crash report trace... but it doesn't hurt to be safe anyway and guard against nullptr symbols. Also safer memory management via unique_ptrs, which fixes a couple of leaks in symbol handling. (cherry-picked from 7973408)
·
final-3_2_3final-3_2_1
1 parent b166e57 commit 850e70b

File tree

6 files changed

+72
-59
lines changed

6 files changed

+72
-59
lines changed
 

‎src/app/qgsprojectproperties.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,10 @@ void QgsProjectProperties::populateStyles()
17611761
for ( int i = 0; i < symbolNames.count(); ++i )
17621762
{
17631763
QString name = symbolNames[i];
1764-
QgsSymbol *symbol = mStyle->symbol( name );
1764+
std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( name ) );
1765+
if ( !symbol )
1766+
continue;
1767+
17651768
QComboBox *cbo = nullptr;
17661769
switch ( symbol->type() )
17671770
{
@@ -1780,10 +1783,9 @@ void QgsProjectProperties::populateStyles()
17801783
}
17811784
if ( cbo )
17821785
{
1783-
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, cbo->iconSize() );
1786+
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), cbo->iconSize() );
17841787
cbo->addItem( icon, name );
17851788
}
1786-
delete symbol;
17871789
}
17881790

17891791
// populate color ramps
@@ -1857,27 +1859,27 @@ void QgsProjectProperties::editSymbol( QComboBox *cbo )
18571859
QMessageBox::information( this, QLatin1String( "" ), tr( "Select a valid symbol" ) );
18581860
return;
18591861
}
1860-
QgsSymbol *symbol = mStyle->symbol( symbolName );
1862+
std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( symbolName ) );
18611863
if ( ! symbol )
18621864
{
18631865
QMessageBox::warning( this, QLatin1String( "" ), tr( "Invalid symbol : " ) + symbolName );
18641866
return;
18651867
}
18661868

18671869
// let the user edit the symbol and update list when done
1868-
QgsSymbolSelectorDialog dlg( symbol, mStyle, nullptr, this );
1870+
QgsSymbolSelectorDialog dlg( symbol.get(), mStyle, nullptr, this );
18691871
if ( dlg.exec() == 0 )
18701872
{
1871-
delete symbol;
18721873
return;
18731874
}
18741875

1875-
// by adding symbol to style with the same name the old effectively gets overwritten
1876-
mStyle->addSymbol( symbolName, symbol );
1877-
18781876
// update icon
1879-
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, cbo->iconSize() );
1877+
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), cbo->iconSize() );
18801878
cbo->setItemIcon( cbo->currentIndex(), icon );
1879+
1880+
// by adding symbol to style with the same name the old effectively gets overwritten
1881+
mStyle->addSymbol( symbolName, symbol.release() );
1882+
18811883
}
18821884

18831885
void QgsProjectProperties::resetPythonMacros()

‎src/core/symbology/qgssymbol.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -266,40 +266,40 @@ void QgsSymbol::setMapUnitScale( const QgsMapUnitScale &scale )
266266

267267
QgsSymbol *QgsSymbol::defaultSymbol( QgsWkbTypes::GeometryType geomType )
268268
{
269-
QgsSymbol *s = nullptr;
269+
std::unique_ptr< QgsSymbol > s;
270270

271271
// override global default if project has a default for this type
272272
QString defaultSymbol;
273273
switch ( geomType )
274274
{
275275
case QgsWkbTypes::PointGeometry :
276-
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Marker" ), QLatin1String( "" ) );
276+
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Marker" ) );
277277
break;
278278
case QgsWkbTypes::LineGeometry :
279-
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Line" ), QLatin1String( "" ) );
279+
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Line" ) );
280280
break;
281281
case QgsWkbTypes::PolygonGeometry :
282-
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Fill" ), QLatin1String( "" ) );
282+
defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Fill" ) );
283283
break;
284284
default:
285285
break;
286286
}
287287
if ( !defaultSymbol.isEmpty() )
288-
s = QgsStyle::defaultStyle()->symbol( defaultSymbol );
288+
s.reset( QgsStyle::defaultStyle()->symbol( defaultSymbol ) );
289289

290290
// if no default found for this type, get global default (as previously)
291-
if ( ! s )
291+
if ( !s )
292292
{
293293
switch ( geomType )
294294
{
295295
case QgsWkbTypes::PointGeometry:
296-
s = new QgsMarkerSymbol();
296+
s = qgis::make_unique< QgsMarkerSymbol >();
297297
break;
298298
case QgsWkbTypes::LineGeometry:
299-
s = new QgsLineSymbol();
299+
s = qgis::make_unique< QgsLineSymbol >();
300300
break;
301301
case QgsWkbTypes::PolygonGeometry:
302-
s = new QgsFillSymbol();
302+
s = qgis::make_unique< QgsFillSymbol >();
303303
break;
304304
default:
305305
QgsDebugMsg( "unknown layer's geometry type" );
@@ -326,7 +326,7 @@ QgsSymbol *QgsSymbol::defaultSymbol( QgsWkbTypes::GeometryType geomType )
326326
s->setColor( QgsApplication::colorSchemeRegistry()->fetchRandomStyleColor() );
327327
}
328328

329-
return s;
329+
return s.release();
330330
}
331331

332332
QgsSymbolLayer *QgsSymbol::symbolLayer( int layer )

‎src/gui/symbology/qgscategorizedsymbolrendererwidget.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -923,14 +923,14 @@ int QgsCategorizedSymbolRendererWidget::matchToSymbols( QgsStyle *style )
923923
for ( int catIdx = 0; catIdx < mRenderer->categories().count(); ++catIdx )
924924
{
925925
QString val = mRenderer->categories().at( catIdx ).value().toString();
926-
QgsSymbol *symbol = style->symbol( val );
926+
std::unique_ptr< QgsSymbol > symbol( style->symbol( val ) );
927927
if ( symbol &&
928928
( ( symbol->type() == QgsSymbol::Marker && mLayer->geometryType() == QgsWkbTypes::PointGeometry )
929929
|| ( symbol->type() == QgsSymbol::Line && mLayer->geometryType() == QgsWkbTypes::LineGeometry )
930930
|| ( symbol->type() == QgsSymbol::Fill && mLayer->geometryType() == QgsWkbTypes::PolygonGeometry ) ) )
931931
{
932932
matched++;
933-
mRenderer->updateCategorySymbol( catIdx, symbol->clone() );
933+
mRenderer->updateCategorySymbol( catIdx, symbol.release() );
934934
}
935935
}
936936
mModel->updateSymbology();

‎src/gui/symbology/qgsstyleexportimportdialog.cpp

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -205,17 +205,18 @@ bool QgsStyleExportImportDialog::populateStyles( QgsStyle *style )
205205
{
206206
name = styleNames[i];
207207
QStringList tags = style->tagsOfSymbol( QgsStyle::SymbolEntity, name );
208-
QgsSymbol *symbol = style->symbol( name );
208+
std::unique_ptr< QgsSymbol > symbol( style->symbol( name ) );
209+
if ( !symbol )
210+
continue;
209211
QStandardItem *item = new QStandardItem( name );
210-
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, listItems->iconSize(), 15 );
212+
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), listItems->iconSize(), 15 );
211213
item->setIcon( icon );
212214
item->setToolTip( QStringLiteral( "<b>%1</b><br><i>%2</i>" ).arg( name, tags.count() > 0 ? tags.join( QStringLiteral( ", " ) ) : tr( "Not tagged" ) ) );
213215
// Set font to 10points to show reasonable text
214216
QFont itemFont = item->font();
215217
itemFont.setPointSize( 10 );
216218
item->setFont( itemFont );
217219
model->appendRow( item );
218-
delete symbol;
219220
}
220221

221222
// and color ramps
@@ -237,10 +238,8 @@ bool QgsStyleExportImportDialog::populateStyles( QgsStyle *style )
237238
void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyle *src, QgsStyle *dst )
238239
{
239240
QString symbolName;
240-
QgsSymbol *symbol = nullptr;
241241
QStringList symbolTags;
242242
bool symbolFavorite;
243-
QgsColorRamp *ramp = nullptr;
244243
QModelIndex index;
245244
bool isSymbol = true;
246245
bool prompt = true;
@@ -255,7 +254,8 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
255254
{
256255
index = selection->at( i );
257256
symbolName = index.model()->data( index, 0 ).toString();
258-
symbol = src->symbol( symbolName );
257+
std::unique_ptr< QgsSymbol > symbol( src->symbol( symbolName ) );
258+
std::unique_ptr< QgsColorRamp > ramp;
259259

260260
if ( !mIgnoreXMLTags->isChecked() )
261261
{
@@ -279,7 +279,7 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
279279
if ( !symbol )
280280
{
281281
isSymbol = false;
282-
ramp = src->colorRamp( symbolName );
282+
ramp.reset( src->colorRamp( symbolName ) );
283283
}
284284

285285
if ( isSymbol )
@@ -297,9 +297,12 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
297297
case QMessageBox::No:
298298
continue;
299299
case QMessageBox::Yes:
300-
dst->addSymbol( symbolName, symbol );
301-
dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
300+
{
301+
QgsSymbol *newSymbol = symbol.get();
302+
dst->addSymbol( symbolName, symbol.release() );
303+
dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
302304
continue;
305+
}
303306
case QMessageBox::YesToAll:
304307
prompt = false;
305308
overwrite = true;
@@ -313,17 +316,21 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
313316

314317
if ( dst->symbolNames().contains( symbolName ) && overwrite )
315318
{
316-
dst->addSymbol( symbolName, symbol );
317-
dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
319+
QgsSymbol *newSymbol = symbol.get();
320+
dst->addSymbol( symbolName, symbol.release() );
321+
dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
322+
continue;
318323
}
319324
else if ( dst->symbolNames().contains( symbolName ) && !overwrite )
320325
{
321326
continue;
322327
}
323328
else
324329
{
325-
dst->addSymbol( symbolName, symbol );
326-
dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
330+
QgsSymbol *newSymbol = symbol.get();
331+
dst->addSymbol( symbolName, symbol.release() );
332+
dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
333+
continue;
327334
}
328335
}
329336
else
@@ -341,9 +348,12 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
341348
case QMessageBox::No:
342349
continue;
343350
case QMessageBox::Yes:
344-
dst->addColorRamp( symbolName, ramp );
345-
dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
351+
{
352+
QgsColorRamp *newRamp = ramp.get();
353+
dst->addColorRamp( symbolName, ramp.release() );
354+
dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
346355
continue;
356+
}
347357
case QMessageBox::YesToAll:
348358
prompt = false;
349359
overwrite = true;
@@ -357,17 +367,21 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
357367

358368
if ( dst->colorRampNames().contains( symbolName ) && overwrite )
359369
{
360-
dst->addColorRamp( symbolName, ramp );
361-
dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
370+
QgsColorRamp *newRamp = ramp.get();
371+
dst->addColorRamp( symbolName, ramp.release() );
372+
dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
373+
continue;
362374
}
363375
else if ( dst->colorRampNames().contains( symbolName ) && !overwrite )
364376
{
365377
continue;
366378
}
367379
else
368380
{
369-
dst->addColorRamp( symbolName, ramp );
370-
dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
381+
QgsColorRamp *newRamp = ramp.get();
382+
dst->addColorRamp( symbolName, ramp.release() );
383+
dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
384+
continue;
371385
}
372386
}
373387
}

‎src/gui/symbology/qgsstylemanagerdialog.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,20 +279,19 @@ void QgsStyleManagerDialog::populateSymbols( const QStringList &symbolNames, boo
279279
for ( int i = 0; i < symbolNames.count(); ++i )
280280
{
281281
QString name = symbolNames[i];
282-
QgsSymbol *symbol = mStyle->symbol( name );
282+
std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( name ) );
283283
if ( symbol && symbol->type() == type )
284284
{
285285
QStringList tags = mStyle->tagsOfSymbol( QgsStyle::SymbolEntity, name );
286286
QStandardItem *item = new QStandardItem( name );
287-
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, listItems->iconSize(), 18 );
287+
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), listItems->iconSize(), 18 );
288288
item->setIcon( icon );
289289
item->setData( name ); // used to find out original name when user edited the name
290290
item->setCheckable( check );
291291
item->setToolTip( QStringLiteral( "<b>%1</b><br><i>%2</i>" ).arg( name, tags.count() > 0 ? tags.join( QStringLiteral( ", " ) ) : tr( "Not tagged" ) ) );
292292
// add to model
293293
model->appendRow( item );
294294
}
295-
delete symbol;
296295
}
297296
selectedSymbolsChanged( QItemSelection(), QItemSelection() );
298297
symbolSelected( listItems->currentIndex() );
@@ -654,18 +653,17 @@ bool QgsStyleManagerDialog::editSymbol()
654653
if ( symbolName.isEmpty() )
655654
return false;
656655

657-
QgsSymbol *symbol = mStyle->symbol( symbolName );
656+
std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( symbolName ) );
658657

659658
// let the user edit the symbol and update list when done
660-
QgsSymbolSelectorDialog dlg( symbol, mStyle, nullptr, this );
659+
QgsSymbolSelectorDialog dlg( symbol.get(), mStyle, nullptr, this );
661660
if ( dlg.exec() == 0 )
662661
{
663-
delete symbol;
664662
return false;
665663
}
666664

667665
// by adding symbol to style with the same name the old effectively gets overwritten
668-
mStyle->addSymbol( symbolName, symbol, true );
666+
mStyle->addSymbol( symbolName, symbol.release(), true );
669667
mModified = true;
670668
return true;
671669
}
@@ -870,8 +868,9 @@ void QgsStyleManagerDialog::exportSelectedItemsImages( const QString &dir, const
870868
{
871869
QString name = index.data().toString();
872870
QString path = dir + '/' + name + '.' + format;
873-
QgsSymbol *sym = mStyle->symbol( name );
874-
sym->exportImage( path, format, size );
871+
std::unique_ptr< QgsSymbol > sym( mStyle->symbol( name ) );
872+
if ( sym )
873+
sym->exportImage( path, format, size );
875874
}
876875
}
877876

‎src/gui/symbology/qgssymbolslistwidget.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,9 @@ void QgsSymbolsListWidget::populateSymbols( const QStringList &names )
295295

296296
for ( int i = 0; i < names.count(); i++ )
297297
{
298-
QgsSymbol *s = mStyle->symbol( names[i] );
299-
if ( s->type() != mSymbol->type() )
298+
std::unique_ptr< QgsSymbol > s( mStyle->symbol( names[i] ) );
299+
if ( !s || s->type() != mSymbol->type() )
300300
{
301-
delete s;
302301
continue;
303302
}
304303
QStringList tags = mStyle->tagsOfSymbol( QgsStyle::SymbolEntity, names[i] );
@@ -312,11 +311,10 @@ void QgsSymbolsListWidget::populateSymbols( const QStringList &names )
312311
itemFont.setPointSize( 10 );
313312
item->setFont( itemFont );
314313
// create preview icon
315-
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( s, previewSize, 15 );
314+
QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( s.get(), previewSize, 15 );
316315
item->setIcon( icon );
317316
// add to model
318317
model->appendRow( item );
319-
delete s;
320318
}
321319
}
322320

@@ -637,7 +635,10 @@ void QgsSymbolsListWidget::setSymbolFromStyle( const QModelIndex &index )
637635
QString symbolName = index.data( Qt::UserRole ).toString();
638636
lblSymbolName->setText( symbolName );
639637
// get new instance of symbol from style
640-
QgsSymbol *s = mStyle->symbol( symbolName );
638+
std::unique_ptr< QgsSymbol > s( mStyle->symbol( symbolName ) );
639+
if ( !s )
640+
return;
641+
641642
// remove all symbol layers from original symbolgroupsCombo
642643
while ( mSymbol->symbolLayerCount() )
643644
mSymbol->deleteSymbolLayer( 0 );
@@ -649,9 +650,6 @@ void QgsSymbolsListWidget::setSymbolFromStyle( const QModelIndex &index )
649650
}
650651
mSymbol->setOpacity( s->opacity() );
651652

652-
// delete the temporary symbol
653-
delete s;
654-
655653
updateSymbolInfo();
656654
emit changed();
657655
}

0 commit comments

Comments
 (0)
Please sign in to comment.