From 435f8a3ad7104fe28bcb79bf9bb00d1f0b7ee32f Mon Sep 17 00:00:00 2001 From: David Steele Date: Mon, 16 Jun 2025 17:05:35 -0400 Subject: [PATCH] Remove cfgParseOptionDefault() and cfgParseOptionRequired(). It is currently not possible to determine the default of all options knowing just the command. Some defaults are set in cfgLoadUpdateOption() and in an upcoming commit defaults may be based on the value of other options. It would be possible to update parser to provide this information but that will complicate the parser and since the logic is only used to simplify options passed to remotes it does not seem worth the effort. For the same reason cfgParseOptionRequired() can also be removed. --- src/config/parse.c | 61 ------------------------- src/config/parse.h | 6 --- src/protocol/helper.c | 9 +--- test/src/module/config/parseTest.c | 7 --- test/src/module/protocol/protocolTest.c | 1 - 5 files changed, 1 insertion(+), 83 deletions(-) diff --git a/src/config/parse.c b/src/config/parse.c index 87f8de96f..311f71093 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -1232,34 +1232,6 @@ cfgParseOptionalFilterDepend(PackRead *const filter, const Config *const config, FUNCTION_TEST_RETURN_TYPE(CfgParseOptionalFilterDependResult, result); } -/**********************************************************************************************************************************/ -FN_EXTERN const String * -cfgParseOptionDefault(const ConfigCommand commandId, const ConfigOption optionId, const String *const defaultDynamicBin) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(ENUM, commandId); - FUNCTION_TEST_PARAM(ENUM, optionId); - FUNCTION_TEST_PARAM(STRING, defaultDynamicBin); - FUNCTION_TEST_END(); - - ASSERT(commandId < CFG_COMMAND_TOTAL); - ASSERT(optionId < CFG_OPTION_TOTAL); - ASSERT(defaultDynamicBin != NULL); - - const String *result = NULL; - - MEM_CONTEXT_TEMP_BEGIN() - { - CfgParseOptionalRuleState optionalRules = {.defaultDynamicBin = defaultDynamicBin}; - - if (cfgParseOptionalRule(&optionalRules, parseRuleOptionalTypeDefault, commandId, optionId)) - result = optionalRules.defaultRaw; - } - MEM_CONTEXT_TEMP_END(); - - FUNCTION_TEST_RETURN_CONST(STRING, result); -} - /**********************************************************************************************************************************/ FN_EXTERN const char * cfgParseOptionName(const ConfigOption optionId) @@ -1301,39 +1273,6 @@ cfgParseOptionKeyIdxName(const ConfigOption optionId, const unsigned int keyIdx) FUNCTION_TEST_RETURN_CONST(STRINGZ, ruleOption->name); } -/**********************************************************************************************************************************/ -FN_EXTERN bool -cfgParseOptionRequired(const ConfigCommand commandId, const ConfigOption optionId) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(ENUM, commandId); - FUNCTION_TEST_PARAM(ENUM, optionId); - FUNCTION_TEST_END(); - - ASSERT(commandId < CFG_COMMAND_TOTAL); - ASSERT(optionId < CFG_OPTION_TOTAL); - - bool found = false; - bool result = false; - - MEM_CONTEXT_TEMP_BEGIN() - { - CfgParseOptionalRuleState optionalRules = {0}; - - if (cfgParseOptionalRule(&optionalRules, parseRuleOptionalTypeRequired, commandId, optionId)) - { - found = true; - result = optionalRules.required; - } - } - MEM_CONTEXT_TEMP_END(); - - if (found) - FUNCTION_TEST_RETURN(BOOL, result); - - FUNCTION_TEST_RETURN(BOOL, parseRuleOption[optionId].required); -} - /**********************************************************************************************************************************/ FN_EXTERN bool cfgParseOptionSecure(const ConfigOption optionId) diff --git a/src/config/parse.h b/src/config/parse.h index 27350862d..9b5dee9f5 100644 --- a/src/config/parse.h +++ b/src/config/parse.h @@ -107,9 +107,6 @@ typedef struct CfgParseOptionResult FN_EXTERN CfgParseOptionResult cfgParseOption(const String *const optionName, const CfgParseOptionParam param); -// Default value for the option -FN_EXTERN const String *cfgParseOptionDefault(ConfigCommand commandId, ConfigOption optionId, const String *defaultDynamicBin); - // Option name from id FN_EXTERN const char *cfgParseOptionName(ConfigOption optionId); @@ -125,9 +122,6 @@ FN_EXTERN ConfigOptionType cfgParseOptionType(ConfigOption optionId); // Get the underlying data type for an option FN_EXTERN ConfigOptionDataType cfgParseOptionDataType(ConfigOption optionId); -// Is the option required? -FN_EXTERN bool cfgParseOptionRequired(ConfigCommand commandId, ConfigOption optionId); - // Get list of stanzas in the configuration FN_EXTERN StringList *cfgParseStanzaList(void); diff --git a/src/protocol/helper.c b/src/protocol/helper.c index b124275bf..5a868fdd0 100644 --- a/src/protocol/helper.c +++ b/src/protocol/helper.c @@ -526,15 +526,8 @@ protocolRemoteParam(const ProtocolStorageType protocolStorageType, const unsigne { ASSERT(cfgOptionGroupId(optionId) == cfgOptGrpPg); - // Remove unrequired/defaulted pg options when the remote type is repo since they won't be used - if (protocolStorageType == protocolStorageTypeRepo) - { - remove = - !cfgParseOptionRequired(cfgCommand(), optionId) || - cfgParseOptionDefault(cfgCommand(), optionId, cfgBin()) != NULL; - } // Move pg options to host index 0 (key 1) so they will be in the default index on the remote host - else + if (protocolStorageType != protocolStorageTypeRepo) { if (hostIdx != 0) { diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index 0e76c2547..3033ea73b 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -1954,9 +1954,6 @@ testRun(void) TEST_ERROR(cfgOptionIdxStr(cfgOptPgHost, 1), AssertError, "option 'pg2-host' is null but non-null was requested"); TEST_RESULT_UINT(cfgOptionUInt64(cfgOptIoTimeout), 60000, "io-timeout is set"); - TEST_RESULT_BOOL(cfgParseOptionRequired(cfgCmdBackup, cfgOptPgHost), false, "pg-host is not required for backup"); - TEST_RESULT_BOOL(cfgParseOptionRequired(cfgCmdInfo, cfgOptStanza), false, "stanza is not required for info"); - TEST_RESULT_STR_Z(cfgOptionIdxDefaultValue(cfgOptBackupStandby, 0), "n", "backup-standby default is false"); TEST_RESULT_STR_Z(cfgOptionIdxDefaultValue(cfgOptBackupStandby, 0), "n", "backup-standby default is false (again)"); TEST_RESULT_PTR(cfgOptionIdxDefaultValue(cfgOptPgHost, 0), NULL, "pg-host default is NULL"); @@ -1966,10 +1963,6 @@ testRun(void) TEST_RESULT_VOID(cfgOptionSet(cfgOptDbTimeout, cfgSourceDefault, VARINT64(30000)), "set db-timeout default"); TEST_RESULT_STR_Z(cfgOptionIdxDefaultValue(cfgOptDbTimeout, 0), "30", "db-timeout default is 30m"); - TEST_RESULT_STR_Z(cfgParseOptionDefault(cfgCmdBackup, cfgOptPgHost, STRDEF("pgbackrest")), NULL, "default not found"); - TEST_RESULT_STR_Z( - cfgParseOptionDefault(cfgCmdBackup, cfgOptRepoPath, STRDEF("pgbackrest")), "/var/lib/pgbackrest", "default found"); - TEST_RESULT_VOID( cfgOptionIdxSet(cfgOptPgSocketPath, 1, cfgSourceDefault, VARSTRDEF("/default")), "set pg-socket-path default"); TEST_RESULT_STR_Z(cfgOptionIdxStr(cfgOptPgSocketPath, 0), "@socket", "pg1-socket-path unchanged"); diff --git a/test/src/module/protocol/protocolTest.c b/test/src/module/protocol/protocolTest.c index aeabb8be9..ec385413c 100644 --- a/test/src/module/protocol/protocolTest.c +++ b/test/src/module/protocol/protocolTest.c @@ -414,7 +414,6 @@ testRun(void) hrnCfgArgRawZ(argList, cfgOptStanza, "test1"); hrnCfgArgRawBool(argList, cfgOptLogSubprocess, true); hrnCfgArgRawZ(argList, cfgOptPgPath, "/unused"); // Will be passed to remote (required) - hrnCfgArgRawZ(argList, cfgOptPgPort, "777"); // Not passed to remote (required but has default) hrnCfgArgRawZ(argList, cfgOptRepoHost, "repo-host"); hrnCfgArgRawZ(argList, cfgOptRepoHostPort, "444"); hrnCfgArgRawZ(argList, cfgOptRepoHostConfig, "/path/pgbackrest.conf");