Skip to content

Commit 6db46f7

Browse files
nyalldawsonnyalldawson
committedNov 20, 2017
[ogr] Safer layer memory management
Use unique_ptr with custom deleter to ensure that QgsOgrProviderUtils::release is called
·
ltr-3_40final-3_0_0
1 parent 163b2e2 commit 6db46f7

File tree

2 files changed

+125
-141
lines changed

2 files changed

+125
-141
lines changed
 

‎src/providers/ogr/qgsogrprovider.cpp

Lines changed: 82 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,11 @@ void QgsOgrProvider::repack()
199199
QString errCause;
200200
if ( mLayerName.isNull() )
201201
{
202-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause );
202+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause ).release();
203203
}
204204
else
205205
{
206-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause );
206+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause ).release();
207207
}
208208
209209
if ( !mOgrOrigLayer )
@@ -299,24 +299,23 @@ QgsVectorLayerExporter::ExportError QgsOgrProvider::createEmptyLayer( const QStr
299299
}
300300
}
301301
302-
QgsVectorFileWriter *writer = new QgsVectorFileWriter(
303-
uri, encoding, fields, wkbType,
304-
srs, driverName, dsOptions, layerOptions, nullptr,
305-
QgsVectorFileWriter::NoSymbology, nullptr,
306-
layerName, action );
302+
std::unique_ptr< QgsVectorFileWriter > writer = qgis::make_unique< QgsVectorFileWriter >(
303+
uri, encoding, fields, wkbType,
304+
srs, driverName, dsOptions, layerOptions, nullptr,
305+
QgsVectorFileWriter::NoSymbology, nullptr,
306+
layerName, action );
307307
308308
QgsVectorFileWriter::WriterError error = writer->hasError();
309309
if ( error )
310310
{
311311
if ( errorMessage )
312312
*errorMessage += writer->errorMessage();
313313
314-
delete writer;
315314
return ( QgsVectorLayerExporter::ExportError ) error;
316315
}
317316
318317
QMap<int, int> attrIdxMap = writer->attrIdxToOgrIdx();
319-
delete writer;
318+
writer.reset();
320319
321320
if ( oldToNewAttrIdxMap )
322321
{
@@ -527,11 +526,11 @@ bool QgsOgrProvider::setSubsetString( const QString &theSQL, bool updateFeatureC
527526
pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
528527
return false;
529528
}
530-
QgsOgrLayer *newLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer, subsetLayerH, theSQL );
529+
QgsOgrLayerUniquePtr newLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer, subsetLayerH, theSQL );
531530
Q_ASSERT( newLayer );
532531
if ( mOgrLayer != mOgrOrigLayer )
533532
QgsOgrProviderUtils::release( mOgrLayer );
534-
mOgrLayer = newLayer;
533+
mOgrLayer = newLayer.release();
535534
}
536535
else
537536
{
@@ -840,17 +839,15 @@ QStringList QgsOgrProvider::subLayers() const
840839
for ( unsigned int i = 0; i < layerCount() ; i++ )
841840
{
842841
QString errCause;
843-
QgsOgrLayer *layer = QgsOgrProviderUtils::getLayer( mOgrOrigLayer->datasetName(),
844-
mOgrOrigLayer->updateMode(),
845-
mOgrOrigLayer->options(),
846-
i,
847-
errCause );
842+
QgsOgrLayerUniquePtr layer = QgsOgrProviderUtils::getLayer( mOgrOrigLayer->datasetName(),
843+
mOgrOrigLayer->updateMode(),
844+
mOgrOrigLayer->options(),
845+
i,
846+
errCause );
848847
if ( !layer )
849848
continue;
850849

851-
addSubLayerDetailsToSubLayerList( i, layer );
852-
853-
QgsOgrProviderUtils::release( layer );
850+
addSubLayerDetailsToSubLayerList( i, layer.get() );
854851
}
855852
}
856853
return mSubLayerList;
@@ -1120,7 +1117,7 @@ QgsRectangle QgsOgrProvider::extent() const
11201117
{
11211118
if ( !mExtent )
11221119
{
1123-
mExtent = new OGREnvelope();
1120+
mExtent.reset( new OGREnvelope() );
11241121

11251122
// get the extent_ (envelope) of the layer
11261123
QgsDebugMsg( "Starting get extent" );
@@ -1143,7 +1140,7 @@ QgsRectangle QgsOgrProvider::extent() const
11431140
// TODO: This can be expensive, do we really need it!
11441141
if ( mOgrLayer == mOgrOrigLayer )
11451142
{
1146-
mOgrLayer->GetExtent( mExtent, true );
1143+
mOgrLayer->GetExtent( mExtent.get(), true );
11471144
}
11481145
else
11491146
{
@@ -1210,8 +1207,7 @@ void QgsOgrProvider::updateExtents()
12101207
void QgsOgrProvider::invalidateCachedExtent( bool bForceRecomputeExtent )
12111208
{
12121209
mForceRecomputeExtent = bForceRecomputeExtent;
1213-
delete mExtent;
1214-
mExtent = nullptr;
1210+
mExtent.reset();
12151211
}
12161212

12171213
size_t QgsOgrProvider::layerCount() const
@@ -3158,7 +3154,7 @@ QSet<QVariant> QgsOgrProvider::uniqueValues( int index, int limit ) const
31583154
sql += " ORDER BY " + textEncoding()->fromUnicode( fld.name() ) + " ASC"; // quoting of fieldname produces a syntax error
31593155

31603156
QgsDebugMsg( QString( "SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
3161-
QgsOgrLayer *l = mOgrLayer->ExecuteSQL( sql );
3157+
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
31623158
if ( !l )
31633159
{
31643160
QgsDebugMsg( "Failed to execute SQL" );
@@ -3174,7 +3170,6 @@ QSet<QVariant> QgsOgrProvider::uniqueValues( int index, int limit ) const
31743170
break;
31753171
}
31763172

3177-
QgsOgrProviderUtils::release( l );
31783173
return uniqueValues;
31793174
}
31803175

@@ -3204,7 +3199,7 @@ QStringList QgsOgrProvider::uniqueStringsMatching( int index, const QString &sub
32043199
sql += " ORDER BY " + textEncoding()->fromUnicode( fld.name() ) + " ASC"; // quoting of fieldname produces a syntax error
32053200

32063201
QgsDebugMsg( QString( "SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
3207-
QgsOgrLayer *l = mOgrLayer->ExecuteSQL( sql );
3202+
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
32083203
if ( !l )
32093204
{
32103205
QgsDebugMsg( "Failed to execute SQL" );
@@ -3221,7 +3216,6 @@ QStringList QgsOgrProvider::uniqueStringsMatching( int index, const QString &sub
32213216
break;
32223217
}
32233218

3224-
QgsOgrProviderUtils::release( l );
32253219
return results;
32263220
}
32273221

@@ -3242,7 +3236,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) const
32423236
sql += " WHERE " + textEncoding()->fromUnicode( mSubsetString );
32433237
}
32443238

3245-
QgsOgrLayer *l = mOgrLayer->ExecuteSQL( sql );
3239+
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
32463240
if ( !l )
32473241
{
32483242
QgsDebugMsg( QString( "Failed to execute SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
@@ -3252,14 +3246,10 @@ QVariant QgsOgrProvider::minimumValue( int index ) const
32523246
gdal::ogr_feature_unique_ptr f( l->GetNextFeature() );
32533247
if ( !f )
32543248
{
3255-
QgsOgrProviderUtils::release( l );
32563249
return QVariant();
32573250
}
32583251

32593252
QVariant value = OGR_F_IsFieldSetAndNotNull( f.get(), 0 ) ? convertValue( fld.type(), textEncoding()->toUnicode( OGR_F_GetFieldAsString( f.get(), 0 ) ) ) : QVariant( fld.type() );
3260-
3261-
QgsOgrProviderUtils::release( l );
3262-
32633253
return value;
32643254
}
32653255

@@ -3280,7 +3270,7 @@ QVariant QgsOgrProvider::maximumValue( int index ) const
32803270
sql += " WHERE " + textEncoding()->fromUnicode( mSubsetString );
32813271
}
32823272

3283-
QgsOgrLayer *l = mOgrLayer->ExecuteSQL( sql );
3273+
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
32843274
if ( !l )
32853275
{
32863276
QgsDebugMsg( QString( "Failed to execute SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
@@ -3290,14 +3280,10 @@ QVariant QgsOgrProvider::maximumValue( int index ) const
32903280
gdal::ogr_feature_unique_ptr f( l->GetNextFeature() );
32913281
if ( !f )
32923282
{
3293-
QgsOgrProviderUtils::release( l );
32943283
return QVariant();
32953284
}
32963285

32973286
QVariant value = OGR_F_IsFieldSetAndNotNull( f.get(), 0 ) ? convertValue( fld.type(), textEncoding()->toUnicode( OGR_F_GetFieldAsString( f.get(), 0 ) ) ) : QVariant( fld.type() );
3298-
3299-
QgsOgrProviderUtils::release( l );
3300-
33013287
return value;
33023288
}
33033289

@@ -3850,11 +3836,11 @@ void QgsOgrProvider::open( OpenMode mode )
38503836
// has precedence over the layerid if both are given.
38513837
if ( !mLayerName.isNull() )
38523838
{
3853-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause );
3839+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause ).release();
38543840
}
38553841
else
38563842
{
3857-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause );
3843+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause ).release();
38583844
}
38593845
}
38603846

@@ -3875,11 +3861,11 @@ void QgsOgrProvider::open( OpenMode mode )
38753861
// try to open read-only
38763862
if ( !mLayerName.isNull() )
38773863
{
3878-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
3864+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause ).release();
38793865
}
38803866
else
38813867
{
3882-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
3868+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause ).release();
38833869
}
38843870
}
38853871

@@ -3943,11 +3929,11 @@ void QgsOgrProvider::open( OpenMode mode )
39433929
// try to open read-only
39443930
if ( !mLayerName.isNull() )
39453931
{
3946-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
3932+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause ).release();
39473933
}
39483934
else
39493935
{
3950-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
3936+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause ).release();
39513937
}
39523938

39533939
mWriteAccess = false;
@@ -4130,7 +4116,7 @@ void QgsOgrProviderUtils::invalidateCachedDatasets( const QString &dsName )
41304116
}
41314117
}
41324118

4133-
QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
4119+
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
41344120
int layerIndex,
41354121
QString &errCause )
41364122
{
@@ -4169,7 +4155,7 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
41694155
return getLayer( dsName, false, QStringList(), layerIndex, errCause );
41704156
}
41714157

4172-
QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
4158+
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
41734159
bool updateMode,
41744160
const QStringList &options,
41754161
int layerIndex,
@@ -4239,18 +4225,18 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
42394225
new QgsOgrProviderUtils::DatasetWithLayers;
42404226
ds->hDS = hDS;
42414227

4242-
QgsOgrLayer *QgsOgrLayer = QgsOgrLayer::CreateForLayer(
4243-
ident, layerName, ds, hLayer );
4244-
ds->setLayers[layerName] = QgsOgrLayer;
4228+
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
4229+
ident, layerName, ds, hLayer );
4230+
ds->setLayers[layerName] = layer.get();
42454231

42464232
QList<DatasetWithLayers *> datasetList;
42474233
datasetList.push_back( ds );
42484234
mapSharedDS[ident] = datasetList;
42494235

4250-
return QgsOgrLayer;
4236+
return layer;
42514237
}
42524238

4253-
QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
4239+
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
42544240
const QString &layerName,
42554241
QString &errCause )
42564242
{
@@ -4283,10 +4269,10 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
42834269
return nullptr;
42844270
}
42854271

4286-
QgsOgrLayer *QgsOgrLayer = QgsOgrLayer::CreateForLayer(
4287-
iter.key(), layerName, ds, hLayer );
4288-
ds->setLayers[layerName] = QgsOgrLayer;
4289-
return QgsOgrLayer;
4272+
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
4273+
iter.key(), layerName, ds, hLayer );
4274+
ds->setLayers[layerName] = layer.get();
4275+
return layer;
42904276
}
42914277
}
42924278
}
@@ -4335,7 +4321,7 @@ bool QgsOgrProviderUtils::canUseOpenedDatasets( const QString &dsName )
43354321
}
43364322

43374323

4338-
QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
4324+
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
43394325
bool updateMode,
43404326
const QStringList &options,
43414327
const QString &layerName,
@@ -4390,10 +4376,10 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
43904376
return nullptr;
43914377
}
43924378

4393-
QgsOgrLayer *QgsOgrLayer = QgsOgrLayer::CreateForLayer(
4394-
ident, layerName, ds, hLayer );
4395-
ds->setLayers[layerName] = QgsOgrLayer;
4396-
return QgsOgrLayer;
4379+
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
4380+
ident, layerName, ds, hLayer );
4381+
ds->setLayers[layerName] = layer.get();
4382+
return layer;
43974383
}
43984384
}
43994385

@@ -4423,10 +4409,10 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
44234409

44244410
ds->hDS = hDS;
44254411

4426-
QgsOgrLayer *QgsOgrLayer = QgsOgrLayer::CreateForLayer(
4427-
ident, layerName, ds, hLayer );
4428-
ds->setLayers[layerName] = QgsOgrLayer;
4429-
return QgsOgrLayer;
4412+
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
4413+
ident, layerName, ds, hLayer );
4414+
ds->setLayers[layerName] = layer.get();
4415+
return layer;
44304416
}
44314417

44324418
GDALDatasetH hDS = OpenHelper( dsName, updateMode, options );
@@ -4450,18 +4436,18 @@ QgsOgrLayer *QgsOgrProviderUtils::getLayer( const QString &dsName,
44504436
new QgsOgrProviderUtils::DatasetWithLayers;
44514437
ds->hDS = hDS;
44524438

4453-
QgsOgrLayer *QgsOgrLayer = QgsOgrLayer::CreateForLayer(
4454-
ident, layerName, ds, hLayer );
4455-
ds->setLayers[layerName] = QgsOgrLayer;
4439+
QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
4440+
ident, layerName, ds, hLayer );
4441+
ds->setLayers[layerName] = layer.get();
44564442

44574443
QList<DatasetWithLayers *> datasetList;
44584444
datasetList.push_back( ds );
44594445
mapSharedDS[ident] = datasetList;
44604446

4461-
return QgsOgrLayer;
4447+
return layer;
44624448
}
44634449

4464-
QgsOgrLayer *QgsOgrProviderUtils::getSqlLayer( QgsOgrLayer *baseLayer,
4450+
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getSqlLayer( QgsOgrLayer *baseLayer,
44654451
OGRLayerH hSqlLayer,
44664452
const QString &sql )
44674453
{
@@ -4530,13 +4516,13 @@ QgsOgrLayer::QgsOgrLayer()
45304516
oFDefn.layer = this;
45314517
}
45324518

4533-
QgsOgrLayer *QgsOgrLayer::CreateForLayer(
4519+
QgsOgrLayerUniquePtr QgsOgrLayer::CreateForLayer(
45344520
const QgsOgrProviderUtils::DatasetIdentification &ident,
45354521
const QString &layerName,
45364522
QgsOgrProviderUtils::DatasetWithLayers *ds,
45374523
OGRLayerH hLayer )
45384524
{
4539-
QgsOgrLayer *layer = new QgsOgrLayer;
4525+
QgsOgrLayerUniquePtr layer( new QgsOgrLayer() );
45404526
layer->ident = ident;
45414527
layer->isSqlLayer = false;
45424528
layer->layerName = layerName;
@@ -4550,13 +4536,13 @@ QgsOgrLayer *QgsOgrLayer::CreateForLayer(
45504536
return layer;
45514537
}
45524538

4553-
QgsOgrLayer *QgsOgrLayer::CreateForSql(
4539+
QgsOgrLayerUniquePtr QgsOgrLayer::CreateForSql(
45544540
const QgsOgrProviderUtils::DatasetIdentification &ident,
45554541
const QString &sql,
45564542
QgsOgrProviderUtils::DatasetWithLayers *ds,
45574543
OGRLayerH hLayer )
45584544
{
4559-
QgsOgrLayer *layer = new QgsOgrLayer;
4545+
QgsOgrLayerUniquePtr layer( new QgsOgrLayer() );
45604546
layer->ident = ident;
45614547
layer->isSqlLayer = true;
45624548
layer->sql = sql;
@@ -4739,7 +4725,7 @@ void QgsOgrLayer::ExecuteSQLNoReturn( const QByteArray &sql )
47394725
GDALDatasetReleaseResultSet( ds->hDS, hSqlLayer );
47404726
}
47414727

4742-
QgsOgrLayer *QgsOgrLayer::ExecuteSQL( const QByteArray &sql )
4728+
QgsOgrLayerUniquePtr QgsOgrLayer::ExecuteSQL( const QByteArray &sql )
47434729
{
47444730
QMutexLocker locker( &ds->mutex );
47454731
OGRLayerH hSqlLayer = GDALDatasetExecuteSQL( ds->hDS,
@@ -4816,8 +4802,8 @@ OGRFeatureH QgsOgrFeatureDefn::CreateFeature()
48164802
// ---------------------------------------------------------------------------
48174803

48184804
static
4819-
QgsOgrLayer *LoadDataSourceAndLayer( const QString &uri,
4820-
QString &errCause )
4805+
QgsOgrLayerUniquePtr LoadDataSourceAndLayer( const QString &uri,
4806+
QString &errCause )
48214807
{
48224808
bool isSubLayer;
48234809
int layerIndex;
@@ -4846,14 +4832,14 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
48464832
const QString &styleName, const QString &styleDescription,
48474833
const QString &uiFileContent, bool useAsDefault, QString &errCause )
48484834
{
4849-
QgsOgrLayer *userLayer = LoadDataSourceAndLayer( uri, errCause );
4835+
QgsOgrLayerUniquePtr userLayer = LoadDataSourceAndLayer( uri, errCause );
48504836
if ( !userLayer )
48514837
return false;
48524838

48534839
QMutex *mutex = nullptr;
48544840
OGRLayerH hUserLayer = userLayer->getHandleAndMutex( mutex );
48554841
GDALDatasetH hDS = userLayer->getDatasetHandleAndMutex( mutex );
4856-
mutex->lock();
4842+
QMutexLocker locker( mutex );
48574843

48584844
// check if layer_styles table already exist
48594845
OGRLayerH hLayer = GDALDatasetGetLayerByName( hDS, "layer_styles" );
@@ -4873,7 +4859,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
48734859
{
48744860
errCause = QObject::tr( "Unable to save layer style. It's not possible to create the destination table on the database." );
48754861
mutex->unlock();
4876-
QgsOgrProviderUtils::release( userLayer );
48774862
return false;
48784863
}
48794864
bool ok = true;
@@ -4938,7 +4923,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
49384923
{
49394924
errCause = QObject::tr( "Unable to save layer style. It's not possible to create the destination table on the database." );
49404925
mutex->unlock();
4941-
QgsOgrProviderUtils::release( userLayer );
49424926
return false;
49434927
}
49444928
}
@@ -4996,7 +4980,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
49964980
{
49974981
errCause = QObject::tr( "Operation aborted" );
49984982
mutex->unlock();
4999-
QgsOgrProviderUtils::release( userLayer );
50004983
return false;
50014984
}
50024985
bNew = false;
@@ -5049,7 +5032,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
50495032
bFeatureOK = OGR_L_SetFeature( hLayer, hFeature.get() ) == OGRERR_NONE;
50505033

50515034
mutex->unlock();
5052-
QgsOgrProviderUtils::release( userLayer );
50535035

50545036
if ( !bFeatureOK )
50555037
{
@@ -5063,8 +5045,8 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
50635045

50645046
static
50655047
bool LoadDataSourceLayerStylesAndLayer( const QString &uri,
5066-
QgsOgrLayer *&layerStyles,
5067-
QgsOgrLayer *&userLayer,
5048+
QgsOgrLayerUniquePtr &layerStyles,
5049+
QgsOgrLayerUniquePtr &userLayer,
50685050
QString &errCause )
50695051
{
50705052
bool isSubLayer;
@@ -5098,7 +5080,7 @@ bool LoadDataSourceLayerStylesAndLayer( const QString &uri,
50985080
}
50995081
if ( !userLayer )
51005082
{
5101-
QgsOgrProviderUtils::release( layerStyles );
5083+
layerStyles.reset();
51025084
return false;
51035085
}
51045086
return true;
@@ -5107,8 +5089,8 @@ bool LoadDataSourceLayerStylesAndLayer( const QString &uri,
51075089

51085090
QGISEXTERN QString loadStyle( const QString &uri, QString &errCause )
51095091
{
5110-
QgsOgrLayer *layerStyles = nullptr;
5111-
QgsOgrLayer *userLayer = nullptr;
5092+
QgsOgrLayerUniquePtr layerStyles;
5093+
QgsOgrLayerUniquePtr userLayer;
51125094
if ( !LoadDataSourceLayerStylesAndLayer( uri, layerStyles, userLayer, errCause ) )
51135095
{
51145096
return QLatin1String( "" );
@@ -5118,8 +5100,8 @@ QGISEXTERN QString loadStyle( const QString &uri, QString &errCause )
51185100
OGRLayerH hLayer = layerStyles->getHandleAndMutex( mutex1 );
51195101
QMutex *mutex2 = nullptr;
51205102
OGRLayerH hUserLayer = userLayer->getHandleAndMutex( mutex2 );
5121-
mutex1->lock();
5122-
mutex2->lock();
5103+
QMutexLocker lock1( mutex1 );
5104+
QMutexLocker lock2( mutex2 );
51235105

51245106
QString selectQmlQuery = QStringLiteral( "f_table_schema=''"
51255107
" AND f_table_name=%1"
@@ -5159,10 +5141,6 @@ QGISEXTERN QString loadStyle( const QString &uri, QString &errCause )
51595141
}
51605142
}
51615143

5162-
mutex1->unlock();
5163-
mutex2->unlock();
5164-
QgsOgrProviderUtils::release( layerStyles );
5165-
QgsOgrProviderUtils::release( userLayer );
51665144
return styleQML;
51675145
}
51685146

@@ -5181,7 +5159,7 @@ QGISEXTERN int listStyles( const QString &uri, QStringList &ids, QStringList &na
51815159
subsetString,
51825160
ogrGeometryType );
51835161

5184-
QgsOgrLayer *userLayer;
5162+
QgsOgrLayerUniquePtr userLayer;
51855163
if ( !layerName.isEmpty() )
51865164
{
51875165
userLayer = QgsOgrProviderUtils::getLayer( filePath, layerName, errCause );
@@ -5195,32 +5173,26 @@ QGISEXTERN int listStyles( const QString &uri, QStringList &ids, QStringList &na
51955173
return -1;
51965174
}
51975175

5198-
QgsOgrLayer *layerStyles =
5176+
QgsOgrLayerUniquePtr layerStyles =
51995177
QgsOgrProviderUtils::getLayer( filePath, "layer_styles", errCause );
52005178
if ( !layerStyles )
52015179
{
52025180
QgsMessageLog::logMessage( QObject::tr( "No styles available on DB" ) );
52035181
errCause = QObject::tr( "No styles available on DB" );
5204-
QgsOgrProviderUtils::release( layerStyles );
5205-
QgsOgrProviderUtils::release( userLayer );
52065182
return 0;
52075183
}
52085184

52095185
QMutex *mutex1 = nullptr;
52105186
OGRLayerH hLayer = layerStyles->getHandleAndMutex( mutex1 );
5211-
mutex1->lock();
5187+
QMutexLocker lock1( mutex1 );
52125188
QMutex *mutex2 = nullptr;
52135189
OGRLayerH hUserLayer = userLayer->getHandleAndMutex( mutex2 );
5214-
mutex2->lock();
5190+
QMutexLocker lock2( mutex2 );
52155191

52165192
if ( OGR_L_GetFeatureCount( hLayer, TRUE ) == 0 )
52175193
{
52185194
QgsMessageLog::logMessage( QObject::tr( "No styles available on DB" ) );
52195195
errCause = QObject::tr( "No styles available on DB" );
5220-
mutex1->unlock();
5221-
mutex2->unlock();
5222-
QgsOgrProviderUtils::release( layerStyles );
5223-
QgsOgrProviderUtils::release( userLayer );
52245196
return 0;
52255197
}
52265198

@@ -5292,45 +5264,34 @@ QGISEXTERN int listStyles( const QString &uri, QStringList &ids, QStringList &na
52925264
}
52935265
}
52945266

5295-
mutex1->unlock();
5296-
mutex2->unlock();
5297-
QgsOgrProviderUtils::release( layerStyles );
5298-
QgsOgrProviderUtils::release( userLayer );
5299-
53005267
return numberOfRelatedStyles;
53015268
}
53025269

53035270
QGISEXTERN QString getStyleById( const QString &uri, QString styleId, QString &errCause )
53045271
{
5305-
QgsOgrLayer *layerStyles = nullptr;
5306-
QgsOgrLayer *userLayer = nullptr;
5272+
QgsOgrLayerUniquePtr layerStyles;
5273+
QgsOgrLayerUniquePtr userLayer;
53075274
if ( !LoadDataSourceLayerStylesAndLayer( uri, layerStyles, userLayer, errCause ) )
53085275
{
53095276
return QLatin1String( "" );
53105277
}
53115278

53125279
QMutex *mutex1 = nullptr;
53135280
OGRLayerH hLayer = layerStyles->getHandleAndMutex( mutex1 );
5314-
mutex1->lock();
5281+
QMutexLocker lock1( mutex1 );
53155282

53165283
bool ok;
53175284
int id = styleId.toInt( &ok );
53185285
if ( !ok )
53195286
{
53205287
errCause = QObject::tr( "Invalid style identifier" );
5321-
mutex1->unlock();
5322-
QgsOgrProviderUtils::release( layerStyles );
5323-
QgsOgrProviderUtils::release( userLayer );
53245288
return QLatin1String( "" );
53255289
}
53265290

53275291
gdal::ogr_feature_unique_ptr hFeature( OGR_L_GetFeature( hLayer, id ) );
53285292
if ( !hFeature )
53295293
{
53305294
errCause = QObject::tr( "No style corresponding to style identifier" );
5331-
mutex1->unlock();
5332-
QgsOgrProviderUtils::release( layerStyles );
5333-
QgsOgrProviderUtils::release( userLayer );
53345295
return QLatin1String( "" );
53355296
}
53365297

@@ -5339,10 +5300,6 @@ QGISEXTERN QString getStyleById( const QString &uri, QString styleId, QString &e
53395300
OGR_F_GetFieldAsString( hFeature.get(),
53405301
OGR_FD_GetFieldIndex( hLayerDefn, "styleQML" ) ) ) );
53415302

5342-
mutex1->unlock();
5343-
QgsOgrProviderUtils::release( layerStyles );
5344-
QgsOgrProviderUtils::release( userLayer );
5345-
53465303
return styleQML;
53475304
}
53485305

@@ -5510,3 +5467,8 @@ QGISEXTERN QList<QgsSourceSelectProvider *> *sourceSelectProviders()
55105467
}
55115468

55125469
#endif
5470+
5471+
void QgsOgrLayerReleaser::operator()( QgsOgrLayer *layer )
5472+
{
5473+
QgsOgrProviderUtils::release( layer );
5474+
}

‎src/providers/ogr/qgsogrprovider.h

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ class QgsOgrProvider : public QgsVectorDataProvider
198198
QMap<int, QString> mDefaultValues;
199199

200200
bool mFirstFieldIsFid = false;
201-
mutable OGREnvelope *mExtent = nullptr;
201+
mutable std::unique_ptr< OGREnvelope > mExtent;
202202
bool mForceRecomputeExtent = false;
203203

204204
/**
@@ -285,6 +285,26 @@ class QgsOgrProvider : public QgsVectorDataProvider
285285

286286
};
287287

288+
class QgsOgrLayer;
289+
290+
/**
291+
* Releases a QgsOgrLayer
292+
*/
293+
struct QgsOgrLayerReleaser
294+
{
295+
296+
/**
297+
* Releases a QgsOgrLayer \a layer.
298+
*/
299+
void operator()( QgsOgrLayer *layer );
300+
301+
};
302+
303+
/**
304+
* Scoped QgsOgrLayer.
305+
*/
306+
using QgsOgrLayerUniquePtr = std::unique_ptr< QgsOgrLayer, QgsOgrLayerReleaser>;
307+
288308
/**
289309
\class QgsOgrProviderUtils
290310
\brief Utility class with static methods
@@ -357,32 +377,32 @@ class QgsOgrProviderUtils
357377
static void GDALCloseWrapper( GDALDatasetH mhDS );
358378

359379
//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it doesn't already use that layer. release() should be called when done with the object
360-
static QgsOgrLayer *getLayer( const QString &dsName,
361-
const QString &layerName,
362-
QString &errCause );
380+
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
381+
const QString &layerName,
382+
QString &errCause );
363383

364384

365385
//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer. release() should be called when done with the object
366-
static QgsOgrLayer *getLayer( const QString &dsName,
367-
bool updateMode,
368-
const QStringList &options,
369-
const QString &layerName,
370-
QString &errCause );
386+
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
387+
bool updateMode,
388+
const QStringList &options,
389+
const QString &layerName,
390+
QString &errCause );
371391

372392
//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it doesn't already use that layer. release() should be called when done with the object
373-
static QgsOgrLayer *getLayer( const QString &dsName,
374-
int layerIndex,
375-
QString &errCause );
393+
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
394+
int layerIndex,
395+
QString &errCause );
376396

377397
//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer. release() should be called when done with the object
378-
static QgsOgrLayer *getLayer( const QString &dsName,
379-
bool updateMode,
380-
const QStringList &options,
381-
int layerIndex,
382-
QString &errCause );
398+
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
399+
bool updateMode,
400+
const QStringList &options,
401+
int layerIndex,
402+
QString &errCause );
383403

384404
//! Return a QgsOgrLayer* with a SQL result layer
385-
static QgsOgrLayer *getSqlLayer( QgsOgrLayer *baseLayer, OGRLayerH hSqlLayer, const QString &sql );
405+
static QgsOgrLayerUniquePtr getSqlLayer( QgsOgrLayer *baseLayer, OGRLayerH hSqlLayer, const QString &sql );
386406

387407
//! Release a QgsOgrLayer*
388408
static void release( QgsOgrLayer *&layer );
@@ -436,6 +456,7 @@ class QgsOgrFeatureDefn
436456
OGRFeatureH CreateFeature();
437457
};
438458

459+
439460
/**
440461
\class QgsOgrLayer
441462
\brief Wrap a OGRLayerH object in a thread-safe way
@@ -456,13 +477,13 @@ class QgsOgrLayer
456477
QgsOgrLayer();
457478
~QgsOgrLayer() = default;
458479

459-
static QgsOgrLayer *CreateForLayer(
480+
static QgsOgrLayerUniquePtr CreateForLayer(
460481
const QgsOgrProviderUtils::DatasetIdentification &ident,
461482
const QString &layerName,
462483
QgsOgrProviderUtils::DatasetWithLayers *ds,
463484
OGRLayerH hLayer );
464485

465-
static QgsOgrLayer *CreateForSql(
486+
static QgsOgrLayerUniquePtr CreateForSql(
466487
const QgsOgrProviderUtils::DatasetIdentification &ident,
467488
const QString &sql,
468489
QgsOgrProviderUtils::DatasetWithLayers *ds,
@@ -566,9 +587,10 @@ class QgsOgrLayer
566587
void ExecuteSQLNoReturn( const QByteArray &sql );
567588

568589
//! Wrapper of GDALDatasetExecuteSQL(). Returned layer must be released with QgsOgrProviderUtils::release()
569-
QgsOgrLayer *ExecuteSQL( const QByteArray &sql );
590+
QgsOgrLayerUniquePtr ExecuteSQL( const QByteArray &sql );
570591
};
571592

593+
572594
// clazy:excludeall=qstring-allocations
573595

574596
#endif // QGSOGRPROVIDER_H

0 commit comments

Comments
 (0)
Please sign in to comment.