Skip to content

Commit

Permalink
Emit a CE_Warning when CPLSet[Thread]ConfigOption() is called with a …
Browse files Browse the repository at this point in the history 
…unknown config option, in debug mode
  • Loading branch information
@rouault
rouault committed Nov 9, 2024
1 parent 3982ec4 commit 902c6ea
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 6 deletions.
70 changes: 67 additions & 3 deletions apps/commonutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}

/************************************************************************/
Expand Down
20 changes: 20 additions & 0 deletions autotest/gcore/misc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() == ""


###############################################################################


Expand Down
14 changes: 14 additions & 0 deletions doc/source/user/configoptions.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
65 changes: 62 additions & 3 deletions gcore/gdal_misc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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++)
{
/* --------------------------------------------------------------------
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -3631,7 +3691,6 @@ int CPL_STDCALL GDALGeneralCmdLineProcessor(int nArgc, char ***ppapszArgv,
return -1;
}

CPLSetConfigOption("CPL_DEBUG", papszArgv[iArg + 1]);
iArg += 1;
}

Expand Down
54 changes: 54 additions & 0 deletions port/cpl_conv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() */
/************************************************************************/
Expand Down Expand Up @@ -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<volatile char **>(CSLSetNameValue(
const_cast<char **>(g_papszConfigOptions), pszKey, pszValue));

Expand Down Expand Up @@ -2005,6 +2057,8 @@ void CPL_STDCALL CPLSetThreadLocalConfigOption(const char *pszKey,
if (bMemoryError)
return;

CPLSetConfigOptionDetectUnknownConfigOption(pszKey, pszValue);

papszTLConfigOptions =
CSLSetNameValue(papszTLConfigOptions, pszKey, pszValue);

Expand Down

0 comments on commit 902c6ea

Please sign in to comment.