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)

src/app/qgsprojectproperties.cpp

@@ -1761,7 +1761,10 @@ void QgsProjectProperties::populateStyles()
   for ( int i = 0; i < symbolNames.count(); ++i )
   {
     QString name = symbolNames[i];
-    QgsSymbol *symbol = mStyle->symbol( name );
+    std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( name ) );
+    if ( !symbol )
+      continue;
+
     QComboBox *cbo = nullptr;
     switch ( symbol->type() )
     {
@@ -1780,10 +1783,9 @@ void QgsProjectProperties::populateStyles()
     }
     if ( cbo )
     {
-      QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, cbo->iconSize() );
+      QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), cbo->iconSize() );
       cbo->addItem( icon, name );
     }
-    delete symbol;
   }
 
   // populate color ramps
@@ -1857,27 +1859,27 @@ void QgsProjectProperties::editSymbol( QComboBox *cbo )
     QMessageBox::information( this, QLatin1String( "" ), tr( "Select a valid symbol" ) );
     return;
   }
-  QgsSymbol *symbol = mStyle->symbol( symbolName );
+  std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( symbolName ) );
   if ( ! symbol )
   {
     QMessageBox::warning( this, QLatin1String( "" ), tr( "Invalid symbol : " ) + symbolName );
     return;
   }
 
   // let the user edit the symbol and update list when done
-  QgsSymbolSelectorDialog dlg( symbol, mStyle, nullptr, this );
+  QgsSymbolSelectorDialog dlg( symbol.get(), mStyle, nullptr, this );
   if ( dlg.exec() == 0 )
   {
-    delete symbol;
     return;
   }
 
-  // by adding symbol to style with the same name the old effectively gets overwritten
-  mStyle->addSymbol( symbolName, symbol );
-
   // update icon
-  QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, cbo->iconSize() );
+  QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), cbo->iconSize() );
   cbo->setItemIcon( cbo->currentIndex(), icon );
+
+  // by adding symbol to style with the same name the old effectively gets overwritten
+  mStyle->addSymbol( symbolName, symbol.release() );
+
 }
 
 void QgsProjectProperties::resetPythonMacros()

src/core/symbology/qgssymbol.cpp

@@ -266,40 +266,40 @@ void QgsSymbol::setMapUnitScale( const QgsMapUnitScale &scale )
 
 QgsSymbol *QgsSymbol::defaultSymbol( QgsWkbTypes::GeometryType geomType )
 {
-  QgsSymbol *s = nullptr;
+  std::unique_ptr< QgsSymbol > s;
 
   // override global default if project has a default for this type
   QString defaultSymbol;
   switch ( geomType )
   {
     case QgsWkbTypes::PointGeometry :
-      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Marker" ), QLatin1String( "" ) );
+      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Marker" ) );
       break;
     case QgsWkbTypes::LineGeometry :
-      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Line" ), QLatin1String( "" ) );
+      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Line" ) );
       break;
     case QgsWkbTypes::PolygonGeometry :
-      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Fill" ), QLatin1String( "" ) );
+      defaultSymbol = QgsProject::instance()->readEntry( QStringLiteral( "DefaultStyles" ), QStringLiteral( "/Fill" ) );
       break;
     default:
       break;
   }
   if ( !defaultSymbol.isEmpty() )
-    s = QgsStyle::defaultStyle()->symbol( defaultSymbol );
+    s.reset( QgsStyle::defaultStyle()->symbol( defaultSymbol ) );
 
   // if no default found for this type, get global default (as previously)
-  if ( ! s )
+  if ( !s )
   {
     switch ( geomType )
     {
       case QgsWkbTypes::PointGeometry:
-        s = new QgsMarkerSymbol();
+        s = qgis::make_unique< QgsMarkerSymbol >();
         break;
       case QgsWkbTypes::LineGeometry:
-        s = new QgsLineSymbol();
+        s = qgis::make_unique< QgsLineSymbol >();
         break;
       case QgsWkbTypes::PolygonGeometry:
-        s = new QgsFillSymbol();
+        s = qgis::make_unique< QgsFillSymbol >();
         break;
       default:
         QgsDebugMsg( "unknown layer's geometry type" );
@@ -326,7 +326,7 @@ QgsSymbol *QgsSymbol::defaultSymbol( QgsWkbTypes::GeometryType geomType )
     s->setColor( QgsApplication::colorSchemeRegistry()->fetchRandomStyleColor() );
   }
 
-  return s;
+  return s.release();
 }
 
 QgsSymbolLayer *QgsSymbol::symbolLayer( int layer )

src/gui/symbology/qgscategorizedsymbolrendererwidget.cpp

@@ -923,14 +923,14 @@ int QgsCategorizedSymbolRendererWidget::matchToSymbols( QgsStyle *style )
   for ( int catIdx = 0; catIdx < mRenderer->categories().count(); ++catIdx )
   {
     QString val = mRenderer->categories().at( catIdx ).value().toString();
-    QgsSymbol *symbol = style->symbol( val );
+    std::unique_ptr< QgsSymbol > symbol( style->symbol( val ) );
     if ( symbol &&
          ( ( symbol->type() == QgsSymbol::Marker && mLayer->geometryType() == QgsWkbTypes::PointGeometry )
            || ( symbol->type() == QgsSymbol::Line && mLayer->geometryType() == QgsWkbTypes::LineGeometry )
            || ( symbol->type() == QgsSymbol::Fill && mLayer->geometryType() == QgsWkbTypes::PolygonGeometry ) ) )
     {
       matched++;
-      mRenderer->updateCategorySymbol( catIdx, symbol->clone() );
+      mRenderer->updateCategorySymbol( catIdx, symbol.release() );
     }
   }
   mModel->updateSymbology();

src/gui/symbology/qgsstyleexportimportdialog.cpp

@@ -205,17 +205,18 @@ bool QgsStyleExportImportDialog::populateStyles( QgsStyle *style )
   {
     name = styleNames[i];
     QStringList tags = style->tagsOfSymbol( QgsStyle::SymbolEntity, name );
-    QgsSymbol *symbol = style->symbol( name );
+    std::unique_ptr< QgsSymbol > symbol( style->symbol( name ) );
+    if ( !symbol )
+      continue;
     QStandardItem *item = new QStandardItem( name );
-    QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, listItems->iconSize(), 15 );
+    QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), listItems->iconSize(), 15 );
     item->setIcon( icon );
     item->setToolTip( QStringLiteral( "<b>%1</b><br><i>%2</i>" ).arg( name, tags.count() > 0 ? tags.join( QStringLiteral( ", " ) ) : tr( "Not tagged" ) ) );
     // Set font to 10points to show reasonable text
     QFont itemFont = item->font();
     itemFont.setPointSize( 10 );
     item->setFont( itemFont );
     model->appendRow( item );
-    delete symbol;
   }
 
   // and color ramps
@@ -237,10 +238,8 @@ bool QgsStyleExportImportDialog::populateStyles( QgsStyle *style )
 void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyle *src, QgsStyle *dst )
 {
   QString symbolName;
-  QgsSymbol *symbol = nullptr;
   QStringList symbolTags;
   bool symbolFavorite;
-  QgsColorRamp *ramp = nullptr;
   QModelIndex index;
   bool isSymbol = true;
   bool prompt = true;
@@ -255,7 +254,8 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
   {
     index = selection->at( i );
     symbolName = index.model()->data( index, 0 ).toString();
-    symbol = src->symbol( symbolName );
+    std::unique_ptr< QgsSymbol > symbol( src->symbol( symbolName ) );
+    std::unique_ptr< QgsColorRamp > ramp;
 
     if ( !mIgnoreXMLTags->isChecked() )
     {
@@ -279,7 +279,7 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
     if ( !symbol )
     {
       isSymbol = false;
-      ramp = src->colorRamp( symbolName );
+      ramp.reset( src->colorRamp( symbolName ) );
     }
 
     if ( isSymbol )
@@ -297,9 +297,12 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
           case QMessageBox::No:
             continue;
           case QMessageBox::Yes:
-            dst->addSymbol( symbolName, symbol );
-            dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
+          {
+            QgsSymbol *newSymbol = symbol.get();
+            dst->addSymbol( symbolName, symbol.release() );
+            dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
             continue;
+          }
           case QMessageBox::YesToAll:
             prompt = false;
             overwrite = true;
@@ -313,17 +316,21 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
 
       if ( dst->symbolNames().contains( symbolName ) && overwrite )
       {
-        dst->addSymbol( symbolName, symbol );
-        dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
+        QgsSymbol *newSymbol = symbol.get();
+        dst->addSymbol( symbolName, symbol.release() );
+        dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
+        continue;
       }
       else if ( dst->symbolNames().contains( symbolName ) && !overwrite )
       {
         continue;
       }
       else
       {
-        dst->addSymbol( symbolName, symbol );
-        dst->saveSymbol( symbolName, symbol, symbolFavorite, symbolTags );
+        QgsSymbol *newSymbol = symbol.get();
+        dst->addSymbol( symbolName, symbol.release() );
+        dst->saveSymbol( symbolName, newSymbol, symbolFavorite, symbolTags );
+        continue;
       }
     }
     else
@@ -341,9 +348,12 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
           case QMessageBox::No:
             continue;
           case QMessageBox::Yes:
-            dst->addColorRamp( symbolName, ramp );
-            dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
+          {
+            QgsColorRamp *newRamp = ramp.get();
+            dst->addColorRamp( symbolName, ramp.release() );
+            dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
             continue;
+          }
           case QMessageBox::YesToAll:
             prompt = false;
             overwrite = true;
@@ -357,17 +367,21 @@ void QgsStyleExportImportDialog::moveStyles( QModelIndexList *selection, QgsStyl
 
       if ( dst->colorRampNames().contains( symbolName ) && overwrite )
       {
-        dst->addColorRamp( symbolName, ramp );
-        dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
+        QgsColorRamp *newRamp = ramp.get();
+        dst->addColorRamp( symbolName, ramp.release() );
+        dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
+        continue;
       }
       else if ( dst->colorRampNames().contains( symbolName ) && !overwrite )
       {
         continue;
       }
       else
       {
-        dst->addColorRamp( symbolName, ramp );
-        dst->saveColorRamp( symbolName, ramp, symbolFavorite, symbolTags );
+        QgsColorRamp *newRamp = ramp.get();
+        dst->addColorRamp( symbolName, ramp.release() );
+        dst->saveColorRamp( symbolName, newRamp, symbolFavorite, symbolTags );
+        continue;
       }
     }
   }

src/gui/symbology/qgsstylemanagerdialog.cpp

@@ -279,20 +279,19 @@ void QgsStyleManagerDialog::populateSymbols( const QStringList &symbolNames, boo
   for ( int i = 0; i < symbolNames.count(); ++i )
   {
     QString name = symbolNames[i];
-    QgsSymbol *symbol = mStyle->symbol( name );
+    std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( name ) );
     if ( symbol && symbol->type() == type )
     {
       QStringList tags = mStyle->tagsOfSymbol( QgsStyle::SymbolEntity, name );
       QStandardItem *item = new QStandardItem( name );
-      QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol, listItems->iconSize(), 18 );
+      QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( symbol.get(), listItems->iconSize(), 18 );
       item->setIcon( icon );
       item->setData( name ); // used to find out original name when user edited the name
       item->setCheckable( check );
       item->setToolTip( QStringLiteral( "<b>%1</b><br><i>%2</i>" ).arg( name, tags.count() > 0 ? tags.join( QStringLiteral( ", " ) ) : tr( "Not tagged" ) ) );
       // add to model
       model->appendRow( item );
     }
-    delete symbol;
   }
   selectedSymbolsChanged( QItemSelection(), QItemSelection() );
   symbolSelected( listItems->currentIndex() );
@@ -654,18 +653,17 @@ bool QgsStyleManagerDialog::editSymbol()
   if ( symbolName.isEmpty() )
     return false;
 
-  QgsSymbol *symbol = mStyle->symbol( symbolName );
+  std::unique_ptr< QgsSymbol > symbol( mStyle->symbol( symbolName ) );
 
   // let the user edit the symbol and update list when done
-  QgsSymbolSelectorDialog dlg( symbol, mStyle, nullptr, this );
+  QgsSymbolSelectorDialog dlg( symbol.get(), mStyle, nullptr, this );
   if ( dlg.exec() == 0 )
   {
-    delete symbol;
     return false;
   }
 
   // by adding symbol to style with the same name the old effectively gets overwritten
-  mStyle->addSymbol( symbolName, symbol, true );
+  mStyle->addSymbol( symbolName, symbol.release(), true );
   mModified = true;
   return true;
 }
@@ -870,8 +868,9 @@ void QgsStyleManagerDialog::exportSelectedItemsImages( const QString &dir, const
   {
     QString name = index.data().toString();
     QString path = dir + '/' + name + '.' + format;
-    QgsSymbol *sym = mStyle->symbol( name );
-    sym->exportImage( path, format, size );
+    std::unique_ptr< QgsSymbol > sym( mStyle->symbol( name ) );
+    if ( sym )
+      sym->exportImage( path, format, size );
   }
 }
 

src/gui/symbology/qgssymbolslistwidget.cpp

@@ -295,10 +295,9 @@ void QgsSymbolsListWidget::populateSymbols( const QStringList &names )
 
   for ( int i = 0; i < names.count(); i++ )
   {
-    QgsSymbol *s = mStyle->symbol( names[i] );
-    if ( s->type() != mSymbol->type() )
+    std::unique_ptr< QgsSymbol > s( mStyle->symbol( names[i] ) );
+    if ( !s || s->type() != mSymbol->type() )
     {
-      delete s;
       continue;
     }
     QStringList tags = mStyle->tagsOfSymbol( QgsStyle::SymbolEntity, names[i] );
@@ -312,11 +311,10 @@ void QgsSymbolsListWidget::populateSymbols( const QStringList &names )
     itemFont.setPointSize( 10 );
     item->setFont( itemFont );
     // create preview icon
-    QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( s, previewSize, 15 );
+    QIcon icon = QgsSymbolLayerUtils::symbolPreviewIcon( s.get(), previewSize, 15 );
     item->setIcon( icon );
     // add to model
     model->appendRow( item );
-    delete s;
   }
 }
 
@@ -637,7 +635,10 @@ void QgsSymbolsListWidget::setSymbolFromStyle( const QModelIndex &index )
   QString symbolName = index.data( Qt::UserRole ).toString();
   lblSymbolName->setText( symbolName );
   // get new instance of symbol from style
-  QgsSymbol *s = mStyle->symbol( symbolName );
+  std::unique_ptr< QgsSymbol > s( mStyle->symbol( symbolName ) );
+  if ( !s )
+    return;
+
   // remove all symbol layers from original symbolgroupsCombo
   while ( mSymbol->symbolLayerCount() )
     mSymbol->deleteSymbolLayer( 0 );
@@ -649,9 +650,6 @@ void QgsSymbolsListWidget::setSymbolFromStyle( const QModelIndex &index )
   }
   mSymbol->setOpacity( s->opacity() );
 
-  // delete the temporary symbol
-  delete s;
-
   updateSymbolInfo();
   emit changed();
 }