From 058318f5434853610ec6ac7a20b2ebaf49e07374 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Tue, 21 Nov 2023 18:30:39 +0100 Subject: [PATCH 1/4] enable removal of file handlers --- port/cpl_vsi.h | 7 +++++++ port/cpl_vsi_virtual.h | 3 +-- port/cpl_vsil.cpp | 12 ++++++++++++ port/cpl_vsil_plugin.cpp | 6 ++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/port/cpl_vsi.h b/port/cpl_vsi.h index 80c66795a3bf..47751df4f199 100644 --- a/port/cpl_vsi.h +++ b/port/cpl_vsi.h @@ -719,6 +719,13 @@ void CPL_DLL VSIFreeFilesystemPluginCallbacksStruct( int CPL_DLL VSIInstallPluginHandler( const char *pszPrefix, const VSIFilesystemPluginCallbacksStruct *poCb); +/** + * Unregister a handler previously installed with VSIInstallPluginHandler() on + * the given prefix. + * @since GDAL 3.9 + */ +int CPL_DLL VSIRemovePluginHandler(const char *pszPrefix); + /* ==================================================================== */ /* Time querying. */ /* ==================================================================== */ diff --git a/port/cpl_vsi_virtual.h b/port/cpl_vsi_virtual.h index 7c9b9ca30483..bf41ee6bbdf1 100644 --- a/port/cpl_vsi_virtual.h +++ b/port/cpl_vsi_virtual.h @@ -327,8 +327,7 @@ class CPL_DLL VSIFileManager static VSIFilesystemHandler *GetHandler(const char *); static void InstallHandler(const std::string &osPrefix, VSIFilesystemHandler *); - /* RemoveHandler is never defined. */ - /* static void RemoveHandler( const std::string& osPrefix ); */ + static void RemoveHandler(const std::string& osPrefix); static char **GetPrefixes(); }; diff --git a/port/cpl_vsil.cpp b/port/cpl_vsil.cpp index 3a649d50bf2d..74683b840d7d 100644 --- a/port/cpl_vsil.cpp +++ b/port/cpl_vsil.cpp @@ -3253,6 +3253,18 @@ void VSIFileManager::InstallHandler(const std::string &osPrefix, Get()->oHandlers[osPrefix] = poHandler; } +/************************************************************************/ +/* RemoveHandler() */ +/************************************************************************/ + +void VSIFileManager::RemoveHandler(const std::string& osPrefix) +{ + if (osPrefix == "") + Get()->poDefaultHandler = nullptr; + else + Get()->oHandlers.erase(osPrefix); +} + /************************************************************************/ /* VSICleanupFileManager() */ /************************************************************************/ diff --git a/port/cpl_vsil_plugin.cpp b/port/cpl_vsil_plugin.cpp index 945f878f55d8..f35d0aaf74b4 100644 --- a/port/cpl_vsil_plugin.cpp +++ b/port/cpl_vsil_plugin.cpp @@ -470,6 +470,12 @@ int VSIInstallPluginHandler(const char *pszPrefix, return 0; } +int VSIRemovePluginHandler(const char *pszPrefix) +{ + VSIFileManager::RemoveHandler(pszPrefix); + return 0; +} + VSIFilesystemPluginCallbacksStruct * VSIAllocFilesystemPluginCallbacksStruct(void) { From 2d94587925bb14c2b361a9234aeb68d3a1d1ff1d Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Tue, 21 Nov 2023 18:51:52 +0100 Subject: [PATCH 2/4] format --- port/cpl_vsi_virtual.h | 2 +- port/cpl_vsil.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/port/cpl_vsi_virtual.h b/port/cpl_vsi_virtual.h index bf41ee6bbdf1..68bbee978342 100644 --- a/port/cpl_vsi_virtual.h +++ b/port/cpl_vsi_virtual.h @@ -327,7 +327,7 @@ class CPL_DLL VSIFileManager static VSIFilesystemHandler *GetHandler(const char *); static void InstallHandler(const std::string &osPrefix, VSIFilesystemHandler *); - static void RemoveHandler(const std::string& osPrefix); + static void RemoveHandler(const std::string &osPrefix); static char **GetPrefixes(); }; diff --git a/port/cpl_vsil.cpp b/port/cpl_vsil.cpp index 74683b840d7d..3752005da80f 100644 --- a/port/cpl_vsil.cpp +++ b/port/cpl_vsil.cpp @@ -3257,7 +3257,7 @@ void VSIFileManager::InstallHandler(const std::string &osPrefix, /* RemoveHandler() */ /************************************************************************/ -void VSIFileManager::RemoveHandler(const std::string& osPrefix) +void VSIFileManager::RemoveHandler(const std::string &osPrefix) { if (osPrefix == "") Get()->poDefaultHandler = nullptr; From 7a077a019e6bdc9f6ef5141ce64b1a6d54b7f2d2 Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Sun, 3 Dec 2023 11:10:15 +0100 Subject: [PATCH 3/4] add test, comments --- autotest/cpp/test_cpl.cpp | 61 +++++++++++++++++++++++++++++++++++++++ port/cpl_vsi.h | 4 +++ 2 files changed, 65 insertions(+) diff --git a/autotest/cpp/test_cpl.cpp b/autotest/cpp/test_cpl.cpp index 30b729214a36..328a0ef7edb4 100644 --- a/autotest/cpp/test_cpl.cpp +++ b/autotest/cpp/test_cpl.cpp @@ -4197,6 +4197,67 @@ TEST_F(test_cpl, VSI_plugin_minimal_testing) VSIFCloseL(fp); EXPECT_TRUE(VSIFOpenL("/vsimyplugin/i_dont_exist", "rb") == nullptr); + + // Check that we can remove the handler + VSIRemovePluginHandler("/vsimyplugin/"); + + EXPECT_TRUE(VSIFOpenL("/vsimyplugin/test", "rb") == nullptr); + EXPECT_TRUE(VSIFOpenL("/vsimyplugin/i_dont_exist", "rb") == nullptr); + + // Removing a non-existing handler is a no-op + VSIRemovePluginHandler("/vsimyplugin/"); + VSIRemovePluginHandler("/vsifoobar/"); +} + +TEST_F(test_cpl, VSI_plugin_removal) +{ + // Test removal of multiple handlers + auto psCallbacks1 = VSIAllocFilesystemPluginCallbacksStruct(); + auto psCallbacks2 = VSIAllocFilesystemPluginCallbacksStruct(); + + psCallbacks1->open = [](void *pUserData, const char *pszFilename, + const char *pszAccess) -> void * + { + (void)pUserData; + if (strcmp(pszFilename, "test1") == 0 && strcmp(pszAccess, "rb") == 0) + return const_cast("ok"); + return nullptr; + }; + + psCallbacks2->open = [](void *pUserData, const char *pszFilename, + const char *pszAccess) -> void * + { + (void)pUserData; + if (strcmp(pszFilename, "test2") == 0 && strcmp(pszAccess, "rb") == 0) + return const_cast("ok"); + return nullptr; + }; + + EXPECT_EQ(VSIInstallPluginHandler("/vsimyplugin1/", psCallbacks1), 0); + EXPECT_EQ(VSIInstallPluginHandler("/vsimyplugin2/", psCallbacks2), 0); + VSIFreeFilesystemPluginCallbacksStruct(psCallbacks1); + VSIFreeFilesystemPluginCallbacksStruct(psCallbacks2); + + auto fp1 = VSIFOpenL("/vsimyplugin1/test1", "rb"); + EXPECT_TRUE(fp1 != nullptr); + auto fp2 = VSIFOpenL("/vsimyplugin2/test2", "rb"); + EXPECT_TRUE(fp2 != nullptr); + + EXPECT_TRUE(VSIFOpenL("/vsimyplugin1/test2", "rb") == nullptr); + EXPECT_TRUE(VSIFOpenL("/vsimyplugin2/test1", "rb") == nullptr); + + VSIFCloseL(fp1); + VSIFCloseL(fp2); + + VSIRemovePluginHandler("/vsimyplugin2/"); + EXPECT_TRUE(VSIFOpenL("/vsimyplugin2/test2", "rb") == nullptr); + + auto fp3 = VSIFOpenL("/vsimyplugin1/test1", "rb"); + EXPECT_TRUE(fp3 != nullptr); + VSIFCloseL(fp3); + + VSIRemovePluginHandler("/vsimyplugin1/"); + EXPECT_TRUE(VSIFOpenL("/vsimyplugin1/test1", "rb") == nullptr); } TEST_F(test_cpl, VSI_plugin_advise_read) diff --git a/port/cpl_vsi.h b/port/cpl_vsi.h index 47751df4f199..e14b3cca37e0 100644 --- a/port/cpl_vsi.h +++ b/port/cpl_vsi.h @@ -722,6 +722,10 @@ int CPL_DLL VSIInstallPluginHandler( /** * Unregister a handler previously installed with VSIInstallPluginHandler() on * the given prefix. + * Note: it is generally unsafe to remove a handler while there are still file + * handles opened that are managed by that handler. It is the responsibility of + * the caller to ensure that it calls this function in a situation where it is + * safe to do so. * @since GDAL 3.9 */ int CPL_DLL VSIRemovePluginHandler(const char *pszPrefix); From 0a0e701a18952d6f754e97132ccff24ed517f34c Mon Sep 17 00:00:00 2001 From: Max Gabrielsson Date: Sun, 3 Dec 2023 23:37:53 +0100 Subject: [PATCH 4/4] remove extra test --- autotest/cpp/test_cpl.cpp | 51 --------------------------------------- 1 file changed, 51 deletions(-) diff --git a/autotest/cpp/test_cpl.cpp b/autotest/cpp/test_cpl.cpp index 328a0ef7edb4..b3749fa8b8de 100644 --- a/autotest/cpp/test_cpl.cpp +++ b/autotest/cpp/test_cpl.cpp @@ -4209,57 +4209,6 @@ TEST_F(test_cpl, VSI_plugin_minimal_testing) VSIRemovePluginHandler("/vsifoobar/"); } -TEST_F(test_cpl, VSI_plugin_removal) -{ - // Test removal of multiple handlers - auto psCallbacks1 = VSIAllocFilesystemPluginCallbacksStruct(); - auto psCallbacks2 = VSIAllocFilesystemPluginCallbacksStruct(); - - psCallbacks1->open = [](void *pUserData, const char *pszFilename, - const char *pszAccess) -> void * - { - (void)pUserData; - if (strcmp(pszFilename, "test1") == 0 && strcmp(pszAccess, "rb") == 0) - return const_cast("ok"); - return nullptr; - }; - - psCallbacks2->open = [](void *pUserData, const char *pszFilename, - const char *pszAccess) -> void * - { - (void)pUserData; - if (strcmp(pszFilename, "test2") == 0 && strcmp(pszAccess, "rb") == 0) - return const_cast("ok"); - return nullptr; - }; - - EXPECT_EQ(VSIInstallPluginHandler("/vsimyplugin1/", psCallbacks1), 0); - EXPECT_EQ(VSIInstallPluginHandler("/vsimyplugin2/", psCallbacks2), 0); - VSIFreeFilesystemPluginCallbacksStruct(psCallbacks1); - VSIFreeFilesystemPluginCallbacksStruct(psCallbacks2); - - auto fp1 = VSIFOpenL("/vsimyplugin1/test1", "rb"); - EXPECT_TRUE(fp1 != nullptr); - auto fp2 = VSIFOpenL("/vsimyplugin2/test2", "rb"); - EXPECT_TRUE(fp2 != nullptr); - - EXPECT_TRUE(VSIFOpenL("/vsimyplugin1/test2", "rb") == nullptr); - EXPECT_TRUE(VSIFOpenL("/vsimyplugin2/test1", "rb") == nullptr); - - VSIFCloseL(fp1); - VSIFCloseL(fp2); - - VSIRemovePluginHandler("/vsimyplugin2/"); - EXPECT_TRUE(VSIFOpenL("/vsimyplugin2/test2", "rb") == nullptr); - - auto fp3 = VSIFOpenL("/vsimyplugin1/test1", "rb"); - EXPECT_TRUE(fp3 != nullptr); - VSIFCloseL(fp3); - - VSIRemovePluginHandler("/vsimyplugin1/"); - EXPECT_TRUE(VSIFOpenL("/vsimyplugin1/test1", "rb") == nullptr); -} - TEST_F(test_cpl, VSI_plugin_advise_read) { auto psCallbacks = VSIAllocFilesystemPluginCallbacksStruct();