db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-03-03 14:52:21 +02:00
Code Issues Releases Activity

Centralize enforcement of option constraints.

Browse Source 
Each option type enforced its own constraints but there was a lot of duplication. Centralize the enforcement to remove the duplication.

Also convert the option type assert to a production error. This is unlikely to happen in production but the test is quite cheap so it can't hurt.

Finally, add a NULL check. Most option types can never be NULL.
This commit is contained in:
David Steele 2020-04-26 17:22:37 -04:00
parent 12069ff8f3
commit 987df62ec0
2 changed files with 51 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
src/config/config.c
View File

@ -802,6 +802,37 @@ cfgOptionResetSet(ConfigOption optionId, bool reset)
}
/**********************************************************************************************************************************/
// Helper to enforce contraints when getting options
static Variant *
cfgOptionInternal(ConfigOption optionId, VariantType typeRequested, bool nullAllowed)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(ENUM, optionId);
FUNCTION_TEST_PARAM(ENUM, typeRequested);
FUNCTION_TEST_PARAM(BOOL, nullAllowed);
FUNCTION_TEST_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
// If the option is not NULL then check it is the requested type
Variant *result = configStatic.option[optionId].value;
if (result != NULL)
{
if (varType(result) != typeRequested)
{
THROW_FMT(
AssertError, "option '%s' is type %u but %u was requested", cfgOptionName(optionId), varType(result),
typeRequested);
}
}
// Else check the option is allowed to be NULL
else
CHECK(nullAllowed);
FUNCTION_TEST_RETURN(result);
}
const Variant *
cfgOption(ConfigOption optionId)
{
@ -821,10 +852,7 @@ cfgOptionBool(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeBool);
FUNCTION_LOG_RETURN(BOOL, varBool(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(BOOL, varBool(cfgOptionInternal(optionId, varTypeBool, false)));
}
double
@ -834,10 +862,7 @@ cfgOptionDbl(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeDouble);
FUNCTION_LOG_RETURN(DOUBLE, varDbl(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(DOUBLE, varDbl(cfgOptionInternal(optionId, varTypeDouble, false)));
}
int
@ -847,10 +872,7 @@ cfgOptionInt(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeInt64);
FUNCTION_LOG_RETURN(INT, varIntForce(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(INT, varIntForce(cfgOptionInternal(optionId, varTypeInt64, false)));
}
int64_t
@ -860,10 +882,7 @@ cfgOptionInt64(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeInt64);
FUNCTION_LOG_RETURN(INT64, varInt64(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(INT64, varInt64(cfgOptionInternal(optionId, varTypeInt64, false)));
}
unsigned int
@ -873,10 +892,7 @@ cfgOptionUInt(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeInt64);
FUNCTION_LOG_RETURN(UINT, varUIntForce(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(UINT, varUIntForce(cfgOptionInternal(optionId, varTypeInt64, false)));
}
uint64_t
@ -886,10 +902,7 @@ cfgOptionUInt64(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeInt64);
FUNCTION_LOG_RETURN(UINT64, varUInt64Force(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(UINT64, varUInt64Force(cfgOptionInternal(optionId, varTypeInt64, false)));
}
const KeyValue *
@ -899,10 +912,7 @@ cfgOptionKv(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(varType(configStatic.option[optionId].value) == varTypeKeyValue);
FUNCTION_LOG_RETURN(KEY_VALUE, varKv(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(KEY_VALUE, varKv(cfgOptionInternal(optionId, varTypeKeyValue, false)));
}
const VariantList *
@ -912,19 +922,19 @@ cfgOptionLst(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(configStatic.option[optionId].value == NULL || varType(configStatic.option[optionId].value) == varTypeVariantList);
Variant *optionValue = cfgOptionInternal(optionId, varTypeVariantList, true);
if (configStatic.option[optionId].value == NULL)
if (optionValue == NULL)
{
MEM_CONTEXT_BEGIN(configStatic.memContext)
{
configStatic.option[optionId].value = varNewVarLst(varLstNew());
optionValue = varNewVarLst(varLstNew());
configStatic.option[optionId].value = optionValue;
}
MEM_CONTEXT_END();
}
FUNCTION_LOG_RETURN(VARIANT_LIST, varVarLst(configStatic.option[optionId].value));
FUNCTION_LOG_RETURN(VARIANT_LIST, varVarLst(optionValue));
}
const String *
@ -934,15 +944,7 @@ cfgOptionStr(ConfigOption optionId)
FUNCTION_LOG_PARAM(ENUM, optionId);
FUNCTION_LOG_END();
ASSERT(optionId < CFG_OPTION_TOTAL);
ASSERT(configStatic.option[optionId].value == NULL || varType(configStatic.option[optionId].value) == varTypeString);
const String *result = NULL;
if (configStatic.option[optionId].value != NULL)
result = varStr(configStatic.option[optionId].value);
FUNCTION_LOG_RETURN_CONST(STRING, result);
FUNCTION_LOG_RETURN_CONST(STRING, varStr(cfgOptionInternal(optionId, varTypeString, true)));
}
void

30
test/src/module/config/configTest.c
View File

@ -151,21 +151,15 @@ testRun(void)
TEST_RESULT_VOID(cfgOptionSet(cfgOptOnline, cfgSourceParam, varNewStrZ("1")), "set online");
TEST_RESULT_BOOL(cfgOptionBool(cfgOptOnline), true, "online is set");
TEST_RESULT_INT(cfgOptionSource(cfgOptOnline), cfgSourceParam, "online source is set");
TEST_ERROR(
cfgOptionDbl(cfgOptOnline), AssertError,
"assertion 'varType(configStatic.option[optionId].value) == varTypeDouble' failed");
TEST_ERROR(
cfgOptionInt64(cfgOptOnline), AssertError,
"assertion 'varType(configStatic.option[optionId].value) == varTypeInt64' failed");
TEST_ERROR(cfgOptionDbl(cfgOptOnline), AssertError, "option 'online' is type 0 but 1 was requested");
TEST_ERROR(cfgOptionInt64(cfgOptOnline), AssertError, "option 'online' is type 0 but 3 was requested");
TEST_RESULT_VOID(cfgOptionSet(cfgOptCompressLevel, cfgSourceParam, varNewInt64(1)), "set compress-level");
TEST_RESULT_INT(cfgOptionInt(cfgOptCompressLevel), 1, "compress-level is set");
TEST_RESULT_VOID(cfgOptionSet(cfgOptCompressLevel, cfgSourceDefault, varNewStrZ("3")), "set compress-level");
TEST_RESULT_INT(cfgOptionUInt(cfgOptCompressLevel), 3, "compress-level is set");
TEST_RESULT_INT(cfgOptionSource(cfgOptCompressLevel), cfgSourceDefault, "compress source is set");
TEST_ERROR(
cfgOptionBool(cfgOptCompressLevel), AssertError,
"assertion 'varType(configStatic.option[optionId].value) == varTypeBool' failed");
TEST_ERROR(cfgOptionBool(cfgOptCompressLevel), AssertError, "option 'compress-level' is type 3 but 0 was requested");
TEST_RESULT_VOID(
cfgOptionSet(cfgOptArchivePushQueueMax, cfgSourceParam, varNewInt64(999999999999)), "set archive-push-queue-max");
@ -177,9 +171,7 @@ testRun(void)
TEST_RESULT_VOID(cfgOptionSet(cfgOptProtocolTimeout, cfgSourceConfig, varNewStrZ("3.3")), "set protocol-timeout");
TEST_RESULT_DOUBLE(cfgOptionDbl(cfgOptProtocolTimeout), 3.3, "protocol-timeout is set");
TEST_RESULT_INT(cfgOptionSource(cfgOptProtocolTimeout), cfgSourceConfig, "protocol-timeout source is set");
TEST_ERROR(
cfgOptionKv(cfgOptProtocolTimeout), AssertError,
"assertion 'varType(configStatic.option[optionId].value) == varTypeKeyValue' failed");
TEST_ERROR(cfgOptionKv(cfgOptProtocolTimeout), AssertError, "option 'protocol-timeout' is type 1 but 4 was requested");
TEST_RESULT_VOID(cfgOptionSet(cfgOptProtocolTimeout, cfgSourceConfig, NULL), "set protocol-timeout to NULL");
TEST_RESULT_PTR(cfgOption(cfgOptProtocolTimeout), NULL, "protocol-timeout is not set");
@ -189,10 +181,7 @@ testRun(void)
"option 'recovery-option' must be set with KeyValue variant");
TEST_RESULT_VOID(cfgOptionSet(cfgOptRecoveryOption, cfgSourceConfig, varNewKv(kvNew())), "set recovery-option");
TEST_RESULT_INT(varLstSize(kvKeyList(cfgOptionKv(cfgOptRecoveryOption))), 0, "recovery-option is set");
TEST_ERROR(
cfgOptionLst(cfgOptRecoveryOption), AssertError,
"assertion 'configStatic.option[optionId].value == NULL"
" || varType(configStatic.option[optionId].value) == varTypeVariantList' failed");
TEST_ERROR(cfgOptionLst(cfgOptRecoveryOption), AssertError, "option 'recovery-option' is type 4 but 8 was requested");
TEST_RESULT_INT(varLstSize(cfgOptionLst(cfgOptDbInclude)), 0, "db-include defaults to empty");
TEST_ERROR(
@ -200,10 +189,7 @@ testRun(void)
"option 'db-include' must be set with VariantList variant");
TEST_RESULT_VOID(cfgOptionSet(cfgOptDbInclude, cfgSourceConfig, varNewVarLst(varLstNew())), "set db-include");
TEST_RESULT_INT(varLstSize(cfgOptionLst(cfgOptDbInclude)), 0, "db-include is set");
TEST_ERROR(
cfgOptionStr(cfgOptDbInclude), AssertError,
"assertion 'configStatic.option[optionId].value == NULL"
" || varType(configStatic.option[optionId].value) == varTypeString' failed");
TEST_ERROR(cfgOptionStr(cfgOptDbInclude), AssertError, "option 'db-include' is type 8 but 5 was requested");
TEST_RESULT_PTR(cfgOptionStr(cfgOptStanza), NULL, "stanza defaults to null");
TEST_ERROR(
@ -211,9 +197,7 @@ testRun(void)
"option 'stanza' must be set with String variant");
TEST_RESULT_VOID(cfgOptionSet(cfgOptStanza, cfgSourceConfig, varNewStrZ("db")), "set stanza");
TEST_RESULT_STR_Z(cfgOptionStr(cfgOptStanza), "db", "stanza is set");
TEST_ERROR(
cfgOptionInt(cfgOptStanza), AssertError,
"assertion 'varType(configStatic.option[optionId].value) == varTypeInt64' failed");
TEST_ERROR(cfgOptionInt(cfgOptStanza), AssertError, "option 'stanza' is type 5 but 3 was requested");
// -------------------------------------------------------------------------------------------------------------------------
TEST_RESULT_VOID(cfgInit(), "config init resets value");