Merge pull request #4794 from rouault/fix_4791

GDALDataset::MarkSuppressOnClose(): delete auxiliary files (fixes #4791)
OSGeo · Nov 10, 2021 · 0385590 · 0385590
1 parent e5aeaff
commit 0385590
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 22 deletions.
diff --git a/autotest/cpp/test_gdal.cpp b/autotest/cpp/test_gdal.cpp
@@ -1850,15 +1850,23 @@ namespace tut
     template<> template<> void object::test<24>()
     {
         const char* pszFilename = "/vsimem/out.tif";
-        GDALDatasetUniquePtr poDstDS(
-            GDALDriver::FromHandle(
-                GDALGetDriverByName("GTiff"))->Create(pszFilename, 1, 1, 1, GDT_Byte, nullptr));
-        poDstDS->MarkSuppressOnClose();
-        poDstDS->GetRasterBand(1)->Fill(255);
-        poDstDS->FlushCache(true);
-        // All buffers have been flushed, but our dirty block should not have been written
-        // hence the checksum will be 0
-        ensure_equals(GDALChecksumImage(GDALRasterBand::FromHandle(poDstDS->GetRasterBand(1)),0,0,1,1), 0);
+        const char* const apszOptions[] = { "PROFILE=BASELINE", nullptr };
+        {
+            GDALDatasetUniquePtr poDstDS(
+                GDALDriver::FromHandle(
+                    GDALGetDriverByName("GTiff"))->Create(pszFilename, 1, 1, 1, GDT_Byte, apszOptions));
+            poDstDS->SetMetadataItem("FOO", "BAR");
+            poDstDS->MarkSuppressOnClose();
+            poDstDS->GetRasterBand(1)->Fill(255);
+            poDstDS->FlushCache(true);
+            // All buffers have been flushed, but our dirty block should not have been written
+            // hence the checksum will be 0
+            ensure_equals(GDALChecksumImage(GDALRasterBand::FromHandle(poDstDS->GetRasterBand(1)),0,0,1,1), 0);
+        }
+        {
+            VSIStatBufL sStat;
+            ensure(VSIStatL(CPLSPrintf("%s.aux.xml", pszFilename), &sStat) != 0);
+        }
     }
 
 } // namespace tut
diff --git a/gdal/frmts/raw/envidataset.cpp b/gdal/frmts/raw/envidataset.cpp
@@ -308,6 +308,10 @@ ENVIDataset::~ENVIDataset()
     {
         GDALDeinitGCPs(static_cast<int>(m_asGCPs.size()), m_asGCPs.data());
     }
+
+    // Should be called before pszHDRFilename is freed.
+    CleanupPostFileClosing();
+
     CPLFree(pszHDRFilename);
 }
 

diff --git a/gdal/gcore/gdal_priv.h b/gdal/gcore/gdal_priv.h
@@ -437,6 +437,9 @@ class CPL_DLL GDALDataset : public GDALMajorObject
     void  ShareLockWithParentDataset(GDALDataset* poParentDataset);
 
 //! @endcond
+
+    void                CleanupPostFileClosing();
+
     virtual int         CloseDependentDatasets();
 //! @cond Doxygen_Suppress
     int                 ValidateLayerCreationOptions( const char* const* papszLCO );
@@ -599,8 +602,7 @@ class CPL_DLL GDALDataset : public GDALMajorObject
     int           GetShared() const;
     void          MarkAsShared();
 
-    /** Set that the dataset must be deleted on close. */
-    void          MarkSuppressOnClose() { bSuppressOnClose = true; }
+    void          MarkSuppressOnClose();
 
     /** Return open options.
      * @return open options.
@@ -1601,7 +1603,7 @@ class CPL_DLL GDALDriver : public GDALMajorObject
                                            void * pProgressData );
 //! @endcond
     static CPLErr       QuietDelete( const char * pszName,
-                                     const char *const *papszAllowedDrivers = nullptr);
+                                     CSLConstList papszAllowedDrivers = nullptr);
 
 //! @cond Doxygen_Suppress
     static CPLErr       DefaultRename( const char * pszNewName,

diff --git a/gdal/gcore/gdaldataset.cpp b/gdal/gcore/gdaldataset.cpp
@@ -1504,6 +1504,36 @@ void GDALDataset::MarkAsShared()
     }
 }
 
+/************************************************************************/
+/*                            MarkAsShared()                            */
+/************************************************************************/
+
+/** Set that the dataset must be deleted on close. */
+void  GDALDataset::MarkSuppressOnClose()
+{
+    bSuppressOnClose = true;
+}
+
+/************************************************************************/
+/*                        CleanupPostFileClosing()                      */
+/************************************************************************/
+
+/** This method should be called by driver implementations in their destructor,
+ * after having closed all files, but before having freed resources that
+ * are needed for their GetFileList() implementation.
+ * This is used to implement MarkSuppressOnClose behavior.
+ */
+void GDALDataset::CleanupPostFileClosing()
+{
+    if( bSuppressOnClose )
+    {
+        char** papszFileList = GetFileList();
+        for( int i = 0; papszFileList && papszFileList[i]; ++i )
+            VSIUnlink(papszFileList[i]);
+        CSLDestroy(papszFileList);
+    }
+}
+
 /************************************************************************/
 /*                            GetGCPCount()                             */
 /************************************************************************/

diff --git a/gdal/gcore/gdaldriver.cpp b/gdal/gcore/gdaldriver.cpp
@@ -1127,17 +1127,14 @@ GDALDatasetH CPL_STDCALL GDALCreateCopy( GDALDriverH hDriver,
  * @param pszName the dataset name to try and delete.
  * @param papszAllowedDrivers NULL to consider all candidate drivers, or a NULL
  * terminated list of strings with the driver short names that must be
- * considered. (Note: functionality currently broken. Argument considered as NULL)
+ * considered. (Note: implemented only starting with GDAL 3.4.1)
  * @return CE_None if the dataset does not exist, or is deleted without issues.
  */
 
 CPLErr GDALDriver::QuietDelete( const char *pszName,
-                                const char *const *papszAllowedDrivers )
+                                CSLConstList papszAllowedDrivers )
 
 {
-    // FIXME! GDALIdentifyDriver() accepts a file list, not a driver list
-    CPL_IGNORE_RET_VAL(papszAllowedDrivers);
-
     VSIStatBufL sStat;
     const bool bExists =
         VSIStatExL(pszName, &sStat,
@@ -1156,10 +1153,34 @@ CPLErr GDALDriver::QuietDelete( const char *pszName,
         return CE_None;
     }
 
-    CPLPushErrorHandler(CPLQuietErrorHandler);
-    GDALDriver * const poDriver =
-        GDALDriver::FromHandle( GDALIdentifyDriver( pszName, nullptr ) );
-    CPLPopErrorHandler();
+    GDALDriver* poDriver = nullptr;
+    if( papszAllowedDrivers )
+    {
+        GDALOpenInfo oOpenInfo(pszName, GDAL_OF_ALL);
+        for( CSLConstList papszIter = papszAllowedDrivers; *papszIter; ++papszIter )
+        {
+            GDALDriver* poTmpDriver = GDALDriver::FromHandle( GDALGetDriverByName(*papszIter) );
+            if( poTmpDriver )
+            {
+                const bool bIdentifyRes =
+                    poTmpDriver->pfnIdentifyEx ?
+                        poTmpDriver->pfnIdentifyEx(poTmpDriver, &oOpenInfo) > 0:
+                    poTmpDriver->pfnIdentify && poTmpDriver->pfnIdentify(&oOpenInfo) > 0;
+                if( bIdentifyRes )
+                {
+                    poDriver = poTmpDriver;
+                    break;
+                }
+            }
+        }
+    }
+    else
+    {
+        CPLPushErrorHandler(CPLQuietErrorHandler);
+        poDriver =
+            GDALDriver::FromHandle( GDALIdentifyDriver( pszName, nullptr ) );
+        CPLPopErrorHandler();
+    }
 
     if( poDriver == nullptr )
         return CE_None;

diff --git a/gdal/gcore/gdalpamdataset.cpp b/gdal/gcore/gdalpamdataset.cpp
@@ -152,7 +152,12 @@ GDALPamDataset::GDALPamDataset()
 GDALPamDataset::~GDALPamDataset()
 
 {
-    if( nPamFlags & GPF_DIRTY )
+    if( bSuppressOnClose )
+    {
+        if( psPam && psPam->pszPamFilename != nullptr )
+            VSIUnlink(psPam->pszPamFilename);
+    }
+    else if( nPamFlags & GPF_DIRTY )
     {
         CPLDebug( "GDALPamDataset", "In destructor with dirty metadata." );
         GDALPamDataset::TrySaveXML();