From 902c6eaebda99cf119043f5b80fddd78b91d02f2 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 10 Nov 2024 00:03:35 +0100 Subject: [PATCH] Emit a CE_Warning when CPLSet[Thread]ConfigOption() is called with a unknown config option, in debug mode --- apps/commonutils.cpp | 70 +++++++++++++++++++++++++++++-- autotest/gcore/misc.py | 20 +++++++++ doc/source/user/configoptions.rst | 14 +++++++ gcore/gdal_misc.cpp | 65 ++++++++++++++++++++++++++-- port/cpl_conv.cpp | 54 ++++++++++++++++++++++++ 5 files changed, 217 insertions(+), 6 deletions(-) diff --git a/apps/commonutils.cpp b/apps/commonutils.cpp index 3c5e3b29c7a8..664fce1ee545 100644 --- a/apps/commonutils.cpp +++ b/apps/commonutils.cpp @@ -60,13 +60,42 @@ 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) { @@ -74,6 +103,41 @@ void EarlySetConfigOptions(int argc, char **argv) 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; + } + } + } } /************************************************************************/ diff --git a/autotest/gcore/misc.py b/autotest/gcore/misc.py index 62f8b29a5663..c0a85ffb3ff4 100755 --- a/autotest/gcore/misc.py +++ b/autotest/gcore/misc.py @@ -1031,6 +1031,26 @@ def test_misc_general_cmd_line_processor(tmp_path): assert processed == ["program", "2", str(tmp_path / "a_path"), "a_string"] +############################################################################### +# Test detection of unknown configuration options + + +def test_misc_detection_unknown_config_options(): + + with gdal.config_option("CPL_DEBUG", "something_not_off"): + with gdal.quiet_errors(): + gdal.ErrorReset() + with gdal.config_option("unknown", "bar"): + assert ( + gdal.GetLastErrorMsg() + == "CPLSetConfigOption() called with key=unknown, which is unknown of GDAL" + ) + + gdal.ErrorReset() + with gdal.config_option("CPL_TIMESTAMP", "ON"): + assert gdal.GetLastErrorMsg() == "" + + ############################################################################### diff --git a/doc/source/user/configoptions.rst b/doc/source/user/configoptions.rst index 3c9c4b7d8eaa..8e6049f4f556 100644 --- a/doc/source/user/configoptions.rst +++ b/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 of GDAL +----------------------------------------------------------------- + +By default GDAL will not warn if the name of the configuration option is unknown +of it. 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 of GDAL + [...] + .. _gdal_configuration_file: diff --git a/gcore/gdal_misc.cpp b/gcore/gdal_misc.cpp index dfbf7e7fe7d8..973e521f0083 100644 --- a/gcore/gdal_misc.cpp +++ b/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; } diff --git a/port/cpl_conv.cpp b/port/cpl_conv.cpp index f1f68b4e8885..d5f067c31783 100644 --- a/port/cpl_conv.cpp +++ b/port/cpl_conv.cpp @@ -82,6 +82,7 @@ #include "cpl_string.h" #include "cpl_vsi.h" #include "cpl_vsil_curl_priv.h" +#include "cpl_known_config_options.h" #ifdef DEBUG #define OGRAPISPY_ENABLED @@ -1903,6 +1904,55 @@ static void NotifyOtherComponentsConfigOptionChanged(const char *pszKey, } } +/************************************************************************/ +/* CPLSetConfigOptionDetectUnknownConfigOption() */ +/************************************************************************/ + +static void CPLSetConfigOptionDetectUnknownConfigOption(const char *pszKey, + const char *pszValue) +{ + static bool bDebug = []() + { + // Check that apszKnownConfigOptions is correctly sorted with + // STRCASECMP() criterion. + for (size_t i = 1; i < CPL_ARRAYSIZE(apszKnownConfigOptions); ++i) + { + if (STRCASECMP(apszKnownConfigOptions[i - 1], + apszKnownConfigOptions[i]) >= 0) + { + CPLError(CE_Failure, CPLE_AppDefined, + "ERROR: apszKnownConfigOptions[] isn't correctly " + "sorted: %s >= %s", + apszKnownConfigOptions[i - 1], + apszKnownConfigOptions[i]); + } + } + + return CPLTestBool(CPLGetConfigOption("CPL_DEBUG", "OFF")); + }(); + + if (bDebug) + { + if (!std::binary_search(std::begin(apszKnownConfigOptions), + std::end(apszKnownConfigOptions), pszKey, + [](const char *a, const char *b) + { return STRCASECMP(a, b) < 0; })) + { + const char *pszOldValue = CPLGetConfigOption(pszKey, nullptr); + if (!((!pszValue && !pszOldValue) || + (pszValue && pszOldValue && EQUAL(pszValue, pszOldValue)))) + { + CPLError(CE_Warning, CPLE_AppDefined, + "CPLSetConfigOption() called with key=%s, which is " + "unknown of GDAL", + pszKey); + } + } + } + else if (EQUAL(pszKey, "CPL_DEBUG") && pszValue) + bDebug = CPLTestBool(pszValue); +} + /************************************************************************/ /* CPLSetConfigOption() */ /************************************************************************/ @@ -1945,6 +1995,8 @@ void CPL_STDCALL CPLSetConfigOption(const char *pszKey, const char *pszValue) OGRAPISPYCPLSetConfigOption(pszKey, pszValue); #endif + CPLSetConfigOptionDetectUnknownConfigOption(pszKey, pszValue); + g_papszConfigOptions = const_cast(CSLSetNameValue( const_cast(g_papszConfigOptions), pszKey, pszValue)); @@ -2005,6 +2057,8 @@ void CPL_STDCALL CPLSetThreadLocalConfigOption(const char *pszKey, if (bMemoryError) return; + CPLSetConfigOptionDetectUnknownConfigOption(pszKey, pszValue); + papszTLConfigOptions = CSLSetNameValue(papszTLConfigOptions, pszKey, pszValue);