Skip to content
Permalink
Browse files

Code linting: modify "if ( foo() ) { foo()->bar }"-like constructs

cppcheck sometimes detects this as a potential null pointer dereference.
This makes sense, although it is quite unlikely that the return a foo()
might change in between.

To avoid such warnings, and also repeated method calls which can have some
cost, I've run the following refactoring script to store the result of
foo() in a local variable.

```shell
for i in `find ../src -name "*.cpp"`; do python analyze.py $i > $i.tmp; diff $i.tmp $i >/dev/null || (echo "Paching $i"; mv $i.tmp $i); rm -f $i.tmp; done
```

with analyze.py being

```python
import re
import sys

lines = [l[0:-1] if l[-1] == '\n' else l for l in open(sys.argv[1], "rt").readlines()]

if_pattern = re.compile("(^[ ]*)if \( ([a-zA-Z0-9_\.]+)\(\) \)$")

i = 0
while i < len(lines):
    line = lines[i]
    modified = False

    m = if_pattern.match(line)
    if m:
        next_line = lines[i+1]
        indent = m.group(1)
        s = m.group(2) + "()"
        tmpVar = m.group(2).split('.')[-1]
        tmpVar = 'l' + tmpVar[0].upper() + tmpVar[1:]

        if next_line == indent + '{':
            found = False

            # Look in the block after the if() for a pattern where we dereference
            # "foo()"
            j = i + 1
            while lines[j] != indent + '}':
                if lines[j].find(s + '->') >= 0 or lines[j].find('*' + s) >= 0 or lines[j].find(s + '[') >= 0:
                    found = True
                    break
                j += 1

            if found:
                print(indent + 'if ( auto *' + tmpVar + ' = ' + s +  ' )')
                j = i + 1
                while lines[j] != indent + '}':
                    print(lines[j].replace(' ' + s, ' ' + tmpVar).replace('.' + s, '.' + tmpVar).replace('*' + s, '*' + tmpVar).replace('!' + s, '!' + tmpVar))
                    j += 1
                print(lines[j])
                modified = True
                i = j

        else:
            if next_line.find(s) >= 0:
                print(indent + 'if ( auto *' + tmpVar + ' = ' + s +  ' )')
                print(next_line.replace(' ' + s, ' ' + tmpVar).replace('.' + s, '.' + tmpVar).replace('*' + s, '*' + tmpVar).replace('!' + s, '!' + tmpVar))
                modified = True
                i += 1

    if not modified:
        print(line)
    i += 1
```
  • Loading branch information
rouault authored and nyalldawson committed Oct 5, 2020
1 parent a3c4fe4 commit 44466a3322cc49069477ff692453569dc57a8386
Showing with 321 additions and 321 deletions.
  1. +8 −8 src/app/qgisapp.cpp
  2. +4 −4 src/app/qgsmaptooloffsetcurve.cpp
  3. +8 −8 src/app/qgsmaptooltrimextendfeature.cpp
  4. +3 −3 src/core/fieldformatter/qgsrelationreferencefieldformatter.cpp
  5. +2 −2 src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp
  6. +2 −2 src/core/geometry/qgscurvepolygon.cpp
  7. +3 −3 src/core/labeling/qgsvectorlayerlabelprovider.cpp
  8. +4 −4 src/core/layertree/qgslayertreemodellegendnode.cpp
  9. +2 −2 src/core/layout/qgslayoutitem.cpp
  10. +5 −5 src/core/layout/qgslayoutitemmarker.cpp
  11. +3 −3 src/core/layout/qgslayoutitempolygon.cpp
  12. +3 −3 src/core/layout/qgslayoutitempolyline.cpp
  13. +3 −3 src/core/layout/qgslayoutitemshape.cpp
  14. +5 −5 src/core/layout/qgsreportsectionfieldgroup.cpp
  15. +2 −2 src/core/mesh/qgsmeshlayer.cpp
  16. +22 −22 src/core/processing/qgsprocessingparameters.cpp
  17. +2 −2 src/core/processing/qgsprocessingutils.cpp
  18. +11 −11 src/core/qgsapplication.cpp
  19. +2 −2 src/core/qgsconditionalstyle.cpp
  20. +2 −2 src/core/qgsdiagramrenderer.cpp
  21. +2 −2 src/core/qgsfeaturepickermodelbase.cpp
  22. +6 −6 src/core/qgslegendrenderer.cpp
  23. +2 −2 src/core/qgsmaplayerlegend.cpp
  24. +2 −2 src/core/qgsmaprendererjob.cpp
  25. +4 −4 src/core/qgsmapthemecollection.cpp
  26. +10 −10 src/core/qgsvectorlayer.cpp
  27. +7 −7 src/core/qgsvectorlayerjoininfo.cpp
  28. +2 −2 src/core/qgsvirtuallayerdefinitionutils.cpp
  29. +4 −4 src/core/raster/qgscolorrampshader.cpp
  30. +4 −4 src/core/raster/qgsrasterlayer.cpp
  31. +4 −4 src/core/raster/qgssinglebandgrayrenderer.cpp
  32. +4 −4 src/core/raster/qgssinglebandpseudocolorrenderer.cpp
  33. +2 −2 src/core/symbology/qgsinterpolatedlinerenderer.cpp
  34. +2 −2 src/core/symbology/qgsmasksymbollayer.cpp
  35. +2 −2 src/core/vectortile/qgsvectortilebasicrenderer.cpp
  36. +2 −2 src/gui/attributetable/qgsfieldconditionalformatwidget.cpp
  37. +2 −2 src/gui/callouts/qgscalloutwidget.cpp
  38. +2 −2 src/gui/editorwidgets/qgsrelationreferenceconfigdlg.cpp
  39. +4 −4 src/gui/labeling/qgslabelinggui.cpp
  40. +2 −2 src/gui/labeling/qgslabelsettingswidgetbase.cpp
  41. +4 −4 src/gui/processing/qgsprocessingaggregatewidgetwrapper.cpp
  42. +4 −4 src/gui/processing/qgsprocessingfieldmapwidgetwrapper.cpp
  43. +6 −6 src/gui/processing/qgsprocessingwidgetwrapper.cpp
  44. +29 −29 src/gui/processing/qgsprocessingwidgetwrapperimpl.cpp
  45. +6 −6 src/gui/qgscheckablecombobox.cpp
  46. +2 −2 src/gui/qgscollapsiblegroupbox.cpp
  47. +8 −8 src/gui/qgsdataitemguiprovider.cpp
  48. +2 −2 src/gui/qgsformannotation.cpp
  49. +2 −2 src/gui/qgsmaptoolcapture.cpp
  50. +2 −2 src/gui/qgsoptionsdialogbase.cpp
  51. +5 −5 src/gui/qgspropertyassistantwidget.cpp
  52. +2 −2 src/gui/qgsrelationeditorwidget.cpp
  53. +10 −10 src/gui/qgstextformatwidget.cpp
  54. +4 −4 src/gui/qgsuserinputwidget.cpp
  55. +8 −8 src/gui/symbology/qgscategorizedsymbolrendererwidget.cpp
  56. +6 −6 src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp
  57. +8 −8 src/gui/symbology/qgsheatmaprendererwidget.cpp
  58. +6 −6 src/gui/symbology/qgslayerpropertieswidget.cpp
  59. +2 −2 src/gui/symbology/qgspointclusterrendererwidget.cpp
  60. +2 −2 src/gui/symbology/qgspointdisplacementrendererwidget.cpp
  61. +6 −6 src/gui/symbology/qgsrendererwidget.cpp
  62. +2 −2 src/gui/symbology/qgssymbollayerwidget.cpp
  63. +4 −4 src/gui/symbology/qgssymbolselectordialog.cpp
  64. +2 −2 src/gui/symbology/qgssymbolslistwidget.cpp
  65. +4 −4 src/gui/symbology/qgsvectorfieldsymbollayerwidget.cpp
  66. +2 −2 src/gui/tableeditor/qgstableeditorwidget.cpp
  67. +2 −2 src/plugins/grass/qgsgrassmoduleinput.cpp
  68. +3 −3 src/providers/arcgisrest/qgsarcgisservicesourceselect.cpp
  69. +2 −2 src/providers/grass/qgsgrassprovidermodule.cpp
  70. +2 −2 src/providers/mssql/qgsmssqldataitems.cpp
  71. +2 −2 src/providers/oracle/qgsoraclesourceselect.cpp
  72. +3 −3 src/providers/postgres/qgspostgresprovider.cpp
  73. +2 −2 src/quickgui/attributes/qgsquickattributemodel.cpp
  74. +2 −2 src/server/qgsfeaturefilterprovidergroup.cpp
@@ -4186,10 +4186,10 @@ void QgisApp::setupConnections()
{
canvas->setCanvasColor( backgroundColor );
}
if ( mapOverviewCanvas() )
if ( auto *lMapOverviewCanvas = mapOverviewCanvas() )
{
mapOverviewCanvas()->setBackgroundColor( backgroundColor );
mapOverviewCanvas()->refresh();
lMapOverviewCanvas->setBackgroundColor( backgroundColor );
lMapOverviewCanvas->refresh();
}
} );

@@ -7300,13 +7300,13 @@ void QgisApp::dxfExport()
flags = flags | QgsDxfExport::FlagNoMText;
dxfExport.setFlags( flags );

if ( mapCanvas() )
if ( auto *lMapCanvas = mapCanvas() )
{
//extent
if ( d.exportMapExtent() )
{
QgsCoordinateTransform t( mapCanvas()->mapSettings().destinationCrs(), d.crs(), QgsProject::instance() );
dxfExport.setExtent( t.transformBoundingBox( mapCanvas()->extent() ) );
QgsCoordinateTransform t( lMapCanvas->mapSettings().destinationCrs(), d.crs(), QgsProject::instance() );
dxfExport.setExtent( t.transformBoundingBox( lMapCanvas->extent() ) );
}
}

if ( !result.successful )
{
// fallback - use message bar if available, otherwise use a message log
if ( messageBar() )
if ( auto *lMessageBar = messageBar() )
{
messageBar()->pushInfo( title, message );
lMessageBar->pushInfo( title, message );
}
else
{
@@ -86,19 +86,19 @@ void QgsMapToolOffsetCurve::canvasReleaseEvent( QgsMapMouseEvent *e )
QgsPointLocator::Types( QgsPointLocator::Edge | QgsPointLocator::Area ) );
}

if ( match.layer() )
if ( auto *lLayer = match.layer() )
{
mSourceLayer = match.layer();
mSourceLayer = lLayer;
QgsFeature fet;
if ( match.layer()->getFeatures( QgsFeatureRequest( match.featureId() ) ).nextFeature( fet ) )
if ( lLayer->getFeatures( QgsFeatureRequest( match.featureId() ) ).nextFeature( fet ) )
{
mSourceFeature = fet;
mCtrlHeldOnFirstClick = ( e->modifiers() & Qt::ControlModifier ); //no geometry modification if ctrl is pressed
prepareGeometry( match, fet );
mRubberBand = createRubberBand();
if ( mRubberBand )
{
mRubberBand->setToGeometry( mManipulatedGeometry, match.layer() );
mRubberBand->setToGeometry( mManipulatedGeometry, lLayer );
}
mModifiedFeature = fet.id();
createUserInputWidget();
@@ -60,9 +60,9 @@ static bool getPoints( const QgsPointLocator::Match &match, QgsPoint &p1, QgsPoi
const QgsFeatureId fid = match.featureId();
const int vertex = match.vertexIndex();

if ( match.layer() )
if ( auto *lLayer = match.layer() )
{
const QgsGeometry geom = match.layer()->getGeometry( fid );
const QgsGeometry geom = lLayer->getGeometry( fid );

if ( !( geom.isNull() || geom.isEmpty() ) )
{
@@ -250,18 +250,18 @@ void QgsMapToolTrimExtendFeature::canvasReleaseEvent( QgsMapMouseEvent *e )
filter.setLayer( mVlayer );
match = mCanvas->snappingUtils()->snapToMap( mMapPoint, &filter, true );

if ( match.layer() )
if ( auto *lLayer = match.layer() )
{

match.layer()->beginEditCommand( tr( "Trim/Extend feature" ) );
match.layer()->changeGeometry( match.featureId(), mGeom );
lLayer->beginEditCommand( tr( "Trim/Extend feature" ) );
lLayer->changeGeometry( match.featureId(), mGeom );
if ( QgsProject::instance()->topologicalEditing() )
{
match.layer()->addTopologicalPoints( mIntersection );
lLayer->addTopologicalPoints( mIntersection );
mLimitLayer->addTopologicalPoints( mIntersection );
}
match.layer()->endEditCommand();
match.layer()->triggerRepaint();
lLayer->endEditCommand();
lLayer->triggerRepaint();

emit messageEmitted( tr( "Feature trimmed/extended." ) );
}
@@ -197,12 +197,12 @@ QList<QgsVectorLayerRef> QgsRelationReferenceFieldFormatter::layerDependencies(
QVariantList QgsRelationReferenceFieldFormatter::availableValues( const QVariantMap &config, int countLimit, const QgsFieldFormatterContext &context ) const
{
QVariantList values;
if ( context.project() )
if ( auto *lProject = context.project() )
{
const QgsVectorLayer *referencedLayer = context.project()->relationManager()->relation( config[QStringLiteral( "Relation" )].toString() ).referencedLayer();
const QgsVectorLayer *referencedLayer = lProject->relationManager()->relation( config[QStringLiteral( "Relation" )].toString() ).referencedLayer();
if ( referencedLayer )
{
int fieldIndex = context.project()->relationManager()->relation( config[QStringLiteral( "Relation" )].toString() ).referencedFields().first();
int fieldIndex = lProject->relationManager()->relation( config[QStringLiteral( "Relation" )].toString() ).referencedFields().first();
values = qgis::setToList( referencedLayer->uniqueValues( fieldIndex, countLimit ) );
}
}
@@ -213,9 +213,9 @@ QVariantList QgsValueRelationFieldFormatter::availableValues( const QVariantMap
{
QVariantList values;

if ( context.project() )
if ( auto *lProject = context.project() )
{
const QgsVectorLayer *referencedLayer = qobject_cast<QgsVectorLayer *>( context.project()->mapLayer( config[QStringLiteral( "Layer" )].toString() ) );
const QgsVectorLayer *referencedLayer = qobject_cast<QgsVectorLayer *>( lProject->mapLayer( config[QStringLiteral( "Layer" )].toString() ) );
if ( referencedLayer )
{
int fieldIndex = referencedLayer->fields().indexOf( config.value( QStringLiteral( "Key" ) ).toString() );
@@ -417,9 +417,9 @@ QDomElement QgsCurvePolygon::asGml3( QDomDocument &doc, int precision, const QSt
json QgsCurvePolygon::asJsonObject( int precision ) const
{
json coordinates( json::array( ) );
if ( exteriorRing() )
if ( auto *lExteriorRing = exteriorRing() )
{
std::unique_ptr< QgsLineString > exteriorLineString( exteriorRing()->curveToLine() );
std::unique_ptr< QgsLineString > exteriorLineString( lExteriorRing->curveToLine() );
QgsPointSequence exteriorPts;
exteriorLineString->points( exteriorPts );
coordinates.push_back( QgsGeometryUtils::pointsToJson( exteriorPts, precision ) );
@@ -557,10 +557,10 @@ void QgsVectorLayerLabelProvider::drawLabelPrivate( pal::LabelPosition *label, Q
QString txt = lf->text( label->getPartId() );
QFontMetricsF *labelfm = lf->labelFontMetrics();

if ( context.maskIdProvider() )
if ( auto *lMaskIdProvider = context.maskIdProvider() )
{
int maskId = context.maskIdProvider()->maskId( label->getFeaturePart()->layer()->provider()->layerId(),
label->getFeaturePart()->layer()->provider()->providerId() );
int maskId = lMaskIdProvider->maskId( label->getFeaturePart()->layer()->provider()->layerId(),
label->getFeaturePart()->layer()->provider()->providerId() );
context.setCurrentMaskId( maskId );
}

@@ -253,8 +253,8 @@ QgsSymbolLegendNode::QgsSymbolLegendNode( QgsLayerTreeLayer *nodeLayer, const Qg
connect( qobject_cast<QgsVectorLayer *>( nodeLayer->layer() ), &QgsVectorLayer::symbolFeatureCountMapChanged, this, &QgsSymbolLegendNode::updateLabel );
connect( nodeLayer, &QObject::destroyed, this, [ = ]() { mLayerNode = nullptr; } );

if ( mItem.symbol() )
mSymbolUsesMapUnits = ( mItem.symbol()->outputUnit() != QgsUnitTypes::RenderMillimeters );
if ( auto *lSymbol = mItem.symbol() )
mSymbolUsesMapUnits = ( lSymbol->outputUnit() != QgsUnitTypes::RenderMillimeters );
}

Qt::ItemFlags QgsSymbolLegendNode::flags() const
@@ -402,8 +402,8 @@ QgsRenderContext *QgsLayerTreeModelLegendNode::createTemporaryRenderContext() co
double scale = 0.0;
double mupp = 0.0;
int dpi = 0;
if ( model() )
model()->legendMapViewData( &mupp, &dpi, &scale );
if ( auto *lModel = model() )
lModel->legendMapViewData( &mupp, &dpi, &scale );

if ( qgsDoubleNear( mupp, 0.0 ) || dpi == 0 || qgsDoubleNear( scale, 0.0 ) )
return nullptr;
@@ -576,8 +576,8 @@ void QgsLayoutItem::setScenePos( const QPointF destinationPos )
//since setPos does not account for item rotation, use difference between
//current scenePos (which DOES account for rotation) and destination pos
//to calculate how much the item needs to move
if ( parentItem() )
setPos( pos() + ( destinationPos - scenePos() ) + parentItem()->scenePos() );
if ( auto *lParentItem = parentItem() )
setPos( pos() + ( destinationPos - scenePos() ) + lParentItem->scenePos() );
else
setPos( pos() + ( destinationPos - scenePos() ) );
}
@@ -64,20 +64,20 @@ QIcon QgsLayoutItemMarker::icon() const

void QgsLayoutItemMarker::refreshSymbol()
{
if ( layout() )
if ( auto *lLayout = layout() )
{
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->renderContext().dpi() );
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( lLayout, nullptr, lLayout->renderContext().dpi() );

std::unique_ptr< QgsMarkerSymbol > sym( mShapeStyleSymbol->clone() );
sym->setAngle( sym->angle() + mNorthArrowRotation );
sym->startRender( rc );
QRectF bounds = sym->bounds( QPointF( 0, 0 ), rc );
sym->stopRender( rc );
mPoint = QPointF( -bounds.left() * 25.4 / layout()->renderContext().dpi(),
-bounds.top() * 25.4 / layout()->renderContext().dpi() );
mPoint = QPointF( -bounds.left() * 25.4 / lLayout->renderContext().dpi(),
-bounds.top() * 25.4 / lLayout->renderContext().dpi() );
bounds.translate( mPoint );

QgsLayoutSize newSizeMm = QgsLayoutSize( bounds.size() * 25.4 / layout()->renderContext().dpi(), QgsUnitTypes::LayoutMillimeters );
QgsLayoutSize newSizeMm = QgsLayoutSize( bounds.size() * 25.4 / lLayout->renderContext().dpi(), QgsUnitTypes::LayoutMillimeters );
mFixedSize = mLayout->renderContext().measurementConverter().convert( newSizeMm, sizeWithUnits().units() );

attemptResize( mFixedSize );
@@ -80,10 +80,10 @@ void QgsLayoutItemPolygon::createDefaultPolygonStyleSymbol()

void QgsLayoutItemPolygon::refreshSymbol()
{
if ( layout() )
if ( auto *lLayout = layout() )
{
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / layout()->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolygonStyleSymbol.get(), rc );
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( lLayout, nullptr, lLayout->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / lLayout->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolygonStyleSymbol.get(), rc );
}

updateSceneRect();
@@ -109,10 +109,10 @@ void QgsLayoutItemPolyline::createDefaultPolylineStyleSymbol()

void QgsLayoutItemPolyline::refreshSymbol()
{
if ( layout() )
if ( auto *lLayout = layout() )
{
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / layout()->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolylineStyleSymbol.get(), rc );
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( lLayout, nullptr, lLayout->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / lLayout->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mPolylineStyleSymbol.get(), rc );
}

updateSceneRect();
@@ -119,10 +119,10 @@ void QgsLayoutItemShape::setShapeType( QgsLayoutItemShape::Shape type )

void QgsLayoutItemShape::refreshSymbol()
{
if ( layout() )
if ( auto *lLayout = layout() )
{
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( layout(), nullptr, layout()->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / layout()->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mShapeStyleSymbol.get(), rc );
QgsRenderContext rc = QgsLayoutUtils::createRenderContextForLayout( lLayout, nullptr, lLayout->renderContext().dpi() );
mMaxSymbolBleed = ( 25.4 / lLayout->renderContext().dpi() ) * QgsSymbolLayerUtils::estimateMaxSymbolBleed( mShapeStyleSymbol.get(), rc );
}

updateBoundingRect();
@@ -125,12 +125,12 @@ QgsLayout *QgsReportSectionFieldGroup::nextBody( bool &ok )
// no features left for this iteration
mFeatures = QgsFeatureIterator();

if ( footer() )
if ( auto *lFooter = footer() )
{
footer()->reportContext().blockSignals( true );
footer()->reportContext().setLayer( mCoverageLayer.get() );
footer()->reportContext().blockSignals( false );
footer()->reportContext().setFeature( mLastFeature );
lFooter->reportContext().blockSignals( true );
lFooter->reportContext().setLayer( mCoverageLayer.get() );
lFooter->reportContext().blockSignals( false );
lFooter->reportContext().setFeature( mLastFeature );
}
ok = false;
return nullptr;
@@ -712,8 +712,8 @@ QgsMeshDatasetIndex QgsMeshLayer::staticVectorDatasetIndex() const

void QgsMeshLayer::setReferenceTime( const QDateTime &referenceTime )
{
if ( dataProvider() )
mTemporalProperties->setReferenceTime( referenceTime, dataProvider()->temporalCapabilities() );
if ( auto *lDataProvider = dataProvider() )
mTemporalProperties->setReferenceTime( referenceTime, lDataProvider->temporalCapabilities() );
else
mTemporalProperties->setReferenceTime( referenceTime, nullptr );
}

0 comments on commit 44466a3

Please sign in to comment.
You can’t perform that action at this time.