Delete raster provider also if invalid

qgis · Dec 18, 2012 · ffd6508 · ffd6508
1 parent 1ea0f11
commit ffd6508
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 7 deletions.
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -3585,6 +3585,9 @@ bool QgisApp::openLayer( const QString & fileName, bool allowInteractive )
   }
   // TODO - should we really call isValidRasterFileName() before addRasterLayer()
   //        this results in 2 calls to GDALOpen()
+  // I think (Radim) that it is better to test only first if valid,
+  // addRasterLayer() is really trying to add layer and gives error if fails
+  //
   // if ( addRasterLayer( fileName, fileInfo.completeBaseName() ) )
   // {
   //   ok  = true );
@@ -7546,8 +7549,6 @@ QgsRasterLayer* QgisApp::addRasterLayer( QString const & rasterFile, QString con
 
       // The first layer loaded is not useful in that case. The user can select it in
       // the list if he wants to load it.
-      // TODO fix this - provider is not deleted in ~QgsRasterLayer()
-      delete layer->dataProvider();
       delete layer;
       layer = 0;
     }
@@ -7723,8 +7724,6 @@ bool QgisApp::addRasterLayers( QStringList const &theFileNameQStringList, bool g
 
           // The first layer loaded is not useful in that case. The user can select it in
           // the list if he wants to load it.
-          // TODO fix this - provider is not deleted in ~QgsRasterLayer()
-          delete layer->dataProvider();
           delete layer;
           layer = 0;
         }

diff --git a/src/core/raster/qgsrasterlayer.cpp b/src/core/raster/qgsrasterlayer.cpp
@@ -193,8 +193,7 @@ QgsRasterLayer::QgsRasterLayer( const QString & uri,
 QgsRasterLayer::~QgsRasterLayer()
 {
   mValid = false;
-  // TODO fix this - provider is not deleted!
-  //delete mDataProvider; // deleted by pipe
+  // Note: provider and other interfaces are owned and deleted by pipe
 }
 
 //////////////////////////////////////////////////////////
@@ -1603,13 +1602,14 @@ void QgsRasterLayer::setDataProvider( QString const & provider )
   }
   QgsDebugMsg( "Data provider created" );
 
+  // Set data provider into pipe even if not valid so that it is deleted with pipe (with layer)
+  mPipe.set( mDataProvider );
   if ( !mDataProvider->isValid() )
   {
     setError( mDataProvider->error() );
     appendError( ERR( tr( "Provider is not valid (provider: %1, URI: %2" ).arg( mProviderKey ).arg( mDataSource ) ) );
     return;
   }
-  mPipe.set( mDataProvider );
 
   if ( provider == "gdal" )
   {

diff --git a/src/providers/gdal/qgsgdalprovider.cpp b/src/providers/gdal/qgsgdalprovider.cpp
@@ -132,12 +132,16 @@ QgsGdalProvider::QgsGdalProvider( QString const & uri )
     // "ERROR 4: `pok.tif' does not exist in the file system, and is not recognised as a supported dataset name."
     // To avoid this message, we test first if the file exists at all.
     // This should be done better adding static create() method or something like that
+    // TODO: Cannot test for file existence, for example NetCDF with sublayers
+    // is using URI: NETCDF:"/path/to/file.cdf":layer1
+    /*
     if ( !QFile::exists( uri ) )
     {
       QString msg = QString( "File does not exist: %1" ).arg( dataSourceUri() );
       appendError( ERR( msg ) );
       return;
     }
+    */
   }
 
   QString gdalUri = dataSourceUri();