diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 75fb83661..fb0c802e9 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -40,6 +40,14 @@

Improve code documentation in config module.

+ + + + + + +

Add cfgOptionTest() and update cfgOption() calls that are better implemented as cfgOptionTest().

+
diff --git a/src/config/config.c b/src/config/config.c index c28d75b86..bf9e45b2a 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -656,6 +656,16 @@ cfgOptionSource(ConfigOption optionId) return configOptionValue[optionId].source; } +/*********************************************************************************************************************************** +Is the option set? +***********************************************************************************************************************************/ +bool +cfgOptionTest(ConfigOption optionId) +{ + cfgOptionCheck(optionId); + return cfgOptionValid(optionId) && configOptionValue[optionId].value != NULL; +} + /*********************************************************************************************************************************** Is the option valid for this command? ***********************************************************************************************************************************/ diff --git a/src/config/config.h b/src/config/config.h index 6c2799e6f..8ef4ec537 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -42,6 +42,7 @@ unsigned int cfgOptionIndexTotal(ConfigOption optionDefId); const char *cfgOptionName(ConfigOption optionId); bool cfgOptionValid(ConfigOption optionId); +bool cfgOptionTest(ConfigOption optionId); /*********************************************************************************************************************************** Option Source Enum diff --git a/src/config/load.c b/src/config/load.c index 0f8422430..d5882faf2 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -50,7 +50,7 @@ cfgLoadParam(unsigned int argListSize, const char *argList[], String *exe) cfgExeSet(exe); // Set default for repo-host-cmd - if (cfgOptionValid(cfgOptRepoHost) && cfgOption(cfgOptRepoHost) != NULL && + if (cfgOptionValid(cfgOptRepoHost) && cfgOptionTest(cfgOptRepoHost) && cfgOptionSource(cfgOptRepoHostCmd) == cfgSourceDefault) { cfgOptionDefaultSet(cfgOptRepoHostCmd, varNewStr(cfgExe())); @@ -59,9 +59,9 @@ cfgLoadParam(unsigned int argListSize, const char *argList[], String *exe) // Set default for pg-host-cmd if (cfgOptionValid(cfgOptPgHostCmd)) { - for (unsigned int optionIdx = 0; optionIdx <= cfgOptionIndexTotal(cfgOptPgHost); optionIdx++) + for (unsigned int optionIdx = 0; optionIdx < cfgOptionIndexTotal(cfgOptPgHost); optionIdx++) { - if (cfgOption(cfgOptPgHost + optionIdx) != NULL && cfgOptionSource(cfgOptPgHostCmd + optionIdx) == cfgSourceDefault) + if (cfgOptionTest(cfgOptPgHost + optionIdx) && cfgOptionSource(cfgOptPgHostCmd + optionIdx) == cfgSourceDefault) cfgOptionDefaultSet(cfgOptPgHostCmd + optionIdx, varNewStr(cfgExe())); } } diff --git a/src/perl/config.c b/src/perl/config.c index 97a73a071..9b8bc674b 100644 --- a/src/perl/config.c +++ b/src/perl/config.c @@ -39,7 +39,7 @@ perlOptionJson() strCat(result, "\""); // Add a comma if another define will be added - if (cfgOption(optionId) != NULL) + if (cfgOptionTest(optionId)) strCat(result, ","); } @@ -53,7 +53,7 @@ perlOptionJson() strCatFmt(result, "\"reset\":%s", strPtr(varStrForce(varNewBool(true)))); // If has a value - if (cfgOption(optionId) != NULL) + if (cfgOptionTest(optionId)) { // If option is reset, then add a comma separator before setting the value if (cfgOptionReset(optionId)) diff --git a/test/lib/pgBackRestTest/Common/DefineTest.pm b/test/lib/pgBackRestTest/Common/DefineTest.pm index eb88854f9..1ce88af6f 100644 --- a/test/lib/pgBackRestTest/Common/DefineTest.pm +++ b/test/lib/pgBackRestTest/Common/DefineTest.pm @@ -478,7 +478,6 @@ my $oTestDef = &TESTDEF_NAME => 'load', &TESTDEF_TOTAL => 1, &TESTDEF_C => true, - &TESTDEF_CDEF => '-DNO_ERROR -DNO_LOG', &TESTDEF_COVERAGE => { diff --git a/test/src/module/config/configTest.c b/test/src/module/config/configTest.c index 16a5f5031..ce92162b9 100644 --- a/test/src/module/config/configTest.c +++ b/test/src/module/config/configTest.c @@ -106,8 +106,12 @@ testRun() // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_BOOL(cfgOptionValid(cfgOptConfig), false, "valid defaults to false"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptConfig), false, "option not valid for the command"); TEST_RESULT_VOID(cfgOptionValidSet(cfgOptConfig, true), "set valid"); TEST_RESULT_BOOL(cfgOptionValid(cfgOptConfig), true, "valid is set"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptConfig), false, "option valid but value is null"); + TEST_RESULT_VOID(cfgOptionSet(cfgOptConfig, cfgSourceParam, varNewStrZ("cfg")), "set option config"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptConfig), true, "option valid and value not null"); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_PTR(cfgOption(cfgOptOnline), NULL, "online is null"); diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index e31ccfa63..0b692d47f 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -348,7 +348,7 @@ testRun() TEST_RESULT_STR(strPtr(cfgExe()), TEST_BACKREST_EXE, " exe is set"); - TEST_RESULT_PTR(cfgOption(cfgOptConfig), NULL, " config is not set"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptConfig), false, " config is not set"); TEST_RESULT_INT(cfgOptionSource(cfgOptConfig), cfgSourceParam, " config is source param"); TEST_RESULT_BOOL(cfgOptionNegate(cfgOptConfig), true, " config is negated"); TEST_RESULT_INT(cfgOptionSource(cfgOptStanza), cfgSourceParam, " stanza is source param"); @@ -410,13 +410,13 @@ testRun() strPtr(configFile), strPtr(configFile), strPtr(configFile), strPtr(configFile), strPtr(configFile), strPtr(configFile)))); - TEST_RESULT_PTR(cfgOption(cfgOptPgHost), NULL, " pg1-path is not defined"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptPgHost), false, " pg1-path is not set"); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgPath)), "/path/to/db", " pg1-path is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptPgPath), cfgSourceConfig, " pg1-path is source config"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptCompress), false, " compress not is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptCompress), cfgSourceConfig, " compress is source config"); - TEST_RESULT_PTR(cfgOption(cfgOptArchiveCheck), NULL, " archive-check is not set"); - TEST_RESULT_PTR(cfgOption(cfgOptArchiveCopy), NULL, " archive-copy is not set"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptArchiveCheck), false, " archive-check is not set"); + TEST_RESULT_BOOL(cfgOptionTest(cfgOptArchiveCopy), false, " archive-copy is not set"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptRepoHardlink), true, " repo-hardlink is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptRepoHardlink), cfgSourceConfig, " repo-hardlink is source config"); TEST_RESULT_INT(cfgOptionInt(cfgOptCompressLevel), 3, " compress-level is set");