Merge pull request #11230 from rouault/known_config_options

Emit a CE_Warning when CPLSet[Thread]ConfigOption() is called with a unknown config option, in debug mode

.github/workflows/code_checks.yml

@@ -220,3 +220,19 @@ jobs:
           xmllint --schema ./frmts/nitf/data/nitf_spec.xsd ./frmts/nitf/data/nitf_spec.xml --noout
           xmllint --schema ./ogr/ogrsf_frmts/vdv/data/vdv452.xsd ./ogr/ogrsf_frmts/vdv/data/vdv452.xml --noout
           xmllint --schema ./ogr/ogrsf_frmts/gmlas/data/gmlasconf.xsd ./ogr/ogrsf_frmts/gmlas/data/gmlasconf.xml --noout
+
+  check-config-options:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout
+      uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
+    - name: Set up Python
+      uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0
+      with:
+        python-version: 3.8
+    - name: Check cmakelist
+      run: |
+        python scripts/collect_config_options.py
+        git diff
+        git diff --quiet || (echo "You need to run scripts/collect_config_options to update port/cpl_known_config_options.h" && /bin/false)
+

apps/commonutils.cpp

@@ -60,20 +60,84 @@ void EarlySetConfigOptions(int argc, char **argv)
     // OGRRegisterAll(), but we can't call GDALGeneralCmdLineProcessor() or
     // OGRGeneralCmdLineProcessor(), because it needs the drivers to be
     // registered for the --format or --formats options.
+
+    // Start with --debug, so that "my_command --config UNKNOWN_CONFIG_OPTION --debug on"
+    // detects and warns about a unknown config option.
     for (int i = 1; i < argc; i++)
     {
-        if (EQUAL(argv[i], "--config") && i + 2 < argc)
+        if (EQUAL(argv[i], "--config") && i + 1 < argc)
         {
-            CPLSetConfigOption(argv[i + 1], argv[i + 2]);
+            const char *pszArg = argv[i + 1];
+            if (strchr(pszArg, '=') != nullptr)
+            {
+                char *pszKey = nullptr;
+                const char *pszValue = CPLParseNameValue(pszArg, &pszKey);
+                if (pszKey && EQUAL(pszKey, "CPL_DEBUG") && pszValue)
+                {
+                    CPLSetConfigOption(pszKey, pszValue);
+                }
+                CPLFree(pszKey);
+                ++i;
+            }
+            else
+            {
+                if (i + 2 >= argc)
+                {
+                    CPLError(CE_Failure, CPLE_AppDefined,
+                             "--config option given without a key and value "
+                             "argument.");
+                    return;
+                }
 
-            i += 2;
+                if (EQUAL(argv[i + 1], "CPL_DEBUG"))
+                {
+                    CPLSetConfigOption(argv[i + 1], argv[i + 2]);
+                }
+
+                i += 2;
+            }
         }
         else if (EQUAL(argv[i], "--debug") && i + 1 < argc)
         {
             CPLSetConfigOption("CPL_DEBUG", argv[i + 1]);
             i += 1;
         }
     }
+    for (int i = 1; i < argc; i++)
+    {
+        if (EQUAL(argv[i], "--config") && i + 1 < argc)
+        {
+            const char *pszArg = argv[i + 1];
+            if (strchr(pszArg, '=') != nullptr)
+            {
+                char *pszKey = nullptr;
+                const char *pszValue = CPLParseNameValue(pszArg, &pszKey);
+                if (pszKey && !EQUAL(pszKey, "CPL_DEBUG") && pszValue)
+                {
+                    CPLSetConfigOption(pszKey, pszValue);
+                }
+                CPLFree(pszKey);
+                ++i;
+            }
+            else
+            {
+                if (i + 2 >= argc)
+                {
+                    CPLError(CE_Failure, CPLE_AppDefined,
+                             "--config option given without a key and value "
+                             "argument.");
+                    return;
+                }
+
+                if (!EQUAL(argv[i + 1], "CPL_DEBUG"))
+                {
+                    CPLSetConfigOption(argv[i + 1], argv[i + 2]);
+                }
+
+                i += 2;
+            }
+        }
+    }
 }
 
 /************************************************************************/

autotest/cpp/test_cpl.cpp

@@ -1293,16 +1293,26 @@ TEST_F(test_cpl, CPLExpandTilde)
     CPLSetConfigOption("HOME", nullptr);
 }
 
-TEST_F(test_cpl, CPLString_constructors)
+TEST_F(test_cpl, CPLDeclareKnownConfigOption)
 {
-    // CPLString(std::string) constructor
-    ASSERT_STREQ(CPLString(std::string("abc")).c_str(), "abc");
-
-    // CPLString(const char*) constructor
-    ASSERT_STREQ(CPLString("abc").c_str(), "abc");
+    CPLConfigOptionSetter oDebugSetter("CPL_DEBUG", "ON", false);
+    {
+        CPLErrorStateBackuper oErrorStateBackuper(CPLQuietErrorHandler);
+        CPLErrorReset();
+        CPLConfigOptionSetter oDeclaredConfigOptionSetter("UNDECLARED_OPTION",
+                                                          "FOO", false);
+        EXPECT_STREQ(CPLGetLastErrorMsg(),
+                     "Unknown configuration option 'UNDECLARED_OPTION'.");
+    }
+    {
+        CPLDeclareKnownConfigOption("DECLARED_OPTION", nullptr);
 
-    // CPLString(const char*, n) constructor
-    ASSERT_STREQ(CPLString("abc", 1).c_str(), "a");
+        CPLErrorStateBackuper oErrorStateBackuper(CPLQuietErrorHandler);
+        CPLErrorReset();
+        CPLConfigOptionSetter oDeclaredConfigOptionSetter("DECLARED_OPTION",
+                                                          "FOO", false);
+        EXPECT_STREQ(CPLGetLastErrorMsg(), "");
+    }
 }
 
 TEST_F(test_cpl, CPLErrorSetState)

doc/source/user/configoptions.rst

@@ -68,6 +68,20 @@ they can be limited to only the current thread with
 For boolean options, the values YES, TRUE or ON can be used to turn the option on;
 NO, FALSE or OFF to turn it off.
 
+How to detect if the passed configuration option is known to GDAL
+-----------------------------------------------------------------
+
+By default GDAL will not warn if the name of the configuration option is unknown.
+Starting with GDAL 3.11, if you set the :config:`CPL_DEBUG` configuration
+option to ``ON`` (or any value that is not ``OFF``, ``FALSE``, ``NO``), a GDAL
+warning will be emitted for unknown configuration options.
+
+.. code-block:: shell
+
+    $ gdalinfo --config BAD_OPTION=TEST --debug on --version
+    Warning 1: CPLSetConfigOption() called with key=BAD_OPTION, which is unknown to GDAL
+    [...]
+
 
 .. _gdal_configuration_file:
 

gcore/gdal_misc.cpp

@@ -3484,6 +3484,64 @@ int CPL_STDCALL GDALGeneralCmdLineProcessor(int nArgc, char ***ppapszArgv,
     /* ==================================================================== */
     /*      Loop over all arguments.                                        */
     /* ==================================================================== */
+
+    // Start with --debug, so that "my_command --config UNKNOWN_CONFIG_OPTION --debug on"
+    // detects and warns about a unknown config option.
+    for (iArg = 1; iArg < nArgc; iArg++)
+    {
+        if (EQUAL(papszArgv[iArg], "--config") && iArg + 2 < nArgc &&
+            EQUAL(papszArgv[iArg + 1], "CPL_DEBUG"))
+        {
+            if (iArg + 1 >= nArgc)
+            {
+                CPLError(CE_Failure, CPLE_AppDefined,
+                         "--config option given without a key=value argument.");
+                return -1;
+            }
+
+            const char *pszArg = papszArgv[iArg + 1];
+            if (strchr(pszArg, '=') != nullptr)
+            {
+                char *pszKey = nullptr;
+                const char *pszValue = CPLParseNameValue(pszArg, &pszKey);
+                if (pszKey && !EQUAL(pszKey, "CPL_DEBUG") && pszValue)
+                {
+                    CPLSetConfigOption(pszKey, pszValue);
+                }
+                CPLFree(pszKey);
+                ++iArg;
+            }
+            else
+            {
+                if (iArg + 2 >= nArgc)
+                {
+                    CPLError(CE_Failure, CPLE_AppDefined,
+                             "--config option given without a key and value "
+                             "argument.");
+                    return -1;
+                }
+
+                if (!EQUAL(papszArgv[iArg + 1], "CPL_DEBUG"))
+                    CPLSetConfigOption(papszArgv[iArg + 1],
+                                       papszArgv[iArg + 2]);
+
+                iArg += 2;
+            }
+        }
+        else if (EQUAL(papszArgv[iArg], "--debug"))
+        {
+            if (iArg + 1 >= nArgc)
+            {
+                CPLError(CE_Failure, CPLE_AppDefined,
+                         "--debug option given without debug level.");
+                return -1;
+            }
+
+            CPLSetConfigOption("CPL_DEBUG", papszArgv[iArg + 1]);
+            iArg += 1;
+        }
+    }
+
     for (iArg = 1; iArg < nArgc; iArg++)
     {
         /* --------------------------------------------------------------------
@@ -3538,7 +3596,7 @@ int CPL_STDCALL GDALGeneralCmdLineProcessor(int nArgc, char ***ppapszArgv,
             {
                 char *pszKey = nullptr;
                 const char *pszValue = CPLParseNameValue(pszArg, &pszKey);
-                if (pszKey && pszValue)
+                if (pszKey && !EQUAL(pszKey, "CPL_DEBUG") && pszValue)
                 {
                     CPLSetConfigOption(pszKey, pszValue);
                 }
@@ -3555,7 +3613,9 @@ int CPL_STDCALL GDALGeneralCmdLineProcessor(int nArgc, char ***ppapszArgv,
                     return -1;
                 }
 
-                CPLSetConfigOption(papszArgv[iArg + 1], papszArgv[iArg + 2]);
+                if (!EQUAL(papszArgv[iArg + 1], "CPL_DEBUG"))
+                    CPLSetConfigOption(papszArgv[iArg + 1],
+                                       papszArgv[iArg + 2]);
 
                 iArg += 2;
             }
@@ -3631,7 +3691,6 @@ int CPL_STDCALL GDALGeneralCmdLineProcessor(int nArgc, char ***ppapszArgv,
                 return -1;
             }
 
-            CPLSetConfigOption("CPL_DEBUG", papszArgv[iArg + 1]);
             iArg += 1;
         }
 

gcore/gdalabstractbandblockcache.cpp

@@ -204,8 +204,7 @@ void GDALAbstractBandBlockCache::StartDirtyBlockFlushingLog()
     m_nInitialDirtyBlocksInFlushCache = 0;
     if (m_nDirtyBlocks > 0 && CPLIsDefaultErrorHandlerAndCatchDebug())
     {
-        const char *pszDebug = CPLGetConfigOption("CPL_DEBUG", nullptr);
-        if (pszDebug && (EQUAL(pszDebug, "ON") || EQUAL(pszDebug, "GDAL")) &&
+        if (CPLIsDebugEnabled() &&
             CPLGetConfigOption("GDAL_REPORT_DIRTY_BLOCK_FLUSHING", nullptr) ==
                 nullptr)
         {

ogr/ogrsf_frmts/gmlas/ogrgmlasreader.cpp

@@ -349,8 +349,7 @@ void GMLASReader::Context::Dump() const
     CPLDebug("GMLAS", "Context");
     CPLDebug("GMLAS", "  m_nLevel = %d", m_nLevel);
     CPLDebug("GMLAS", "  m_poFeature = %p", m_poFeature);
-    const char *pszDebug = CPLGetConfigOption("CPL_DEBUG", "OFF");
-    if (EQUAL(pszDebug, "ON") || EQUAL(pszDebug, "GMLAS"))
+    if (CPLIsDebugEnabled())
     {
         if (m_poFeature)
             m_poFeature->DumpReadable(stderr);