From f46bfafdd53dd0031a320fa95b08568263901ff3 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 20 Aug 2026 12:27:13 +0700 Subject: [PATCH] Fix option name in block incremental map errors. The error for an invalid value in a block incremental map named the option with cfgParseOptionKeyIdxName(), which expects a key index, but the callers passed a group index. A repository configured as anything but repo1 was named as repo1. Use cfgOptionIdxName() instead, which takes the group index and maps it to the key. The errors for the checksum size map also named the size map option since that option id was passed by mistake. These options are internal so neither error was user-facing. --- src/command/backup/option.c.inc | 16 ++++++++-------- test/src/module/command/backupTest.c | 27 ++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/command/backup/option.c.inc b/src/command/backup/option.c.inc index b2639e343..dba282d39 100644 --- a/src/command/backup/option.c.inc +++ b/src/command/backup/option.c.inc @@ -47,11 +47,11 @@ static const ManifestBlockIncrMap manifestBlockIncrMap = // Convert map size static unsigned int -backupBlockIncrMapSize(const ConfigOption optionId, const unsigned int optionKeyIdx, const String *const value) +backupBlockIncrMapSize(const ConfigOption optionId, const unsigned int optionIdx, const String *const value) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(ENUM, optionId); - FUNCTION_TEST_PARAM(UINT, optionKeyIdx); + FUNCTION_TEST_PARAM(UINT, optionIdx); FUNCTION_TEST_PARAM(STRING, value); FUNCTION_TEST_END(); @@ -69,7 +69,7 @@ backupBlockIncrMapSize(const ConfigOption optionId, const unsigned int optionKey { THROW_FMT( OptionInvalidValueError, "'%s' is not valid for '%s' option", strZ(value), - cfgParseOptionKeyIdxName(optionId, optionKeyIdx)); + cfgOptionIdxName(optionId, optionIdx)); } TRY_END(); @@ -80,11 +80,11 @@ backupBlockIncrMapSize(const ConfigOption optionId, const unsigned int optionKey // Convert map checksum size static unsigned int -backupBlockIncrMapChecksumSize(const ConfigOption optionId, const unsigned int optionKeyIdx, const Variant *const value) +backupBlockIncrMapChecksumSize(const ConfigOption optionId, const unsigned int optionIdx, const Variant *const value) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(ENUM, optionId); - FUNCTION_TEST_PARAM(UINT, optionKeyIdx); + FUNCTION_TEST_PARAM(UINT, optionIdx); FUNCTION_TEST_PARAM(VARIANT, value); FUNCTION_TEST_END(); @@ -103,7 +103,7 @@ backupBlockIncrMapChecksumSize(const ConfigOption optionId, const unsigned int o { THROW_FMT( OptionInvalidValueError, "'%s' is not valid for '%s' option", strZ(varStr(value)), - cfgParseOptionKeyIdxName(optionId, optionKeyIdx)); + cfgOptionIdxName(optionId, optionIdx)); } FUNCTION_TEST_RETURN(UINT, result); @@ -190,9 +190,9 @@ backupBlockIncrMap(void) ManifestBlockIncrChecksumSizeMap manifestBuildBlockIncrChecksumSizeMap = { .blockSize = backupBlockIncrMapSize( - cfgOptRepoBlockSizeMap, cfgOptionIdxDefault(cfgOptRepoBlockChecksumSizeMap), varStr(mapKey)), + cfgOptRepoBlockChecksumSizeMap, cfgOptionIdxDefault(cfgOptRepoBlockChecksumSizeMap), varStr(mapKey)), .checksumSize = backupBlockIncrMapChecksumSize( - cfgOptRepoBlockSizeMap, cfgOptionIdxDefault(cfgOptRepoBlockChecksumSizeMap), + cfgOptRepoBlockChecksumSizeMap, cfgOptionIdxDefault(cfgOptRepoBlockChecksumSizeMap), kvGet(manifestBlockIncrChecksumSizeKv, mapKey)), }; diff --git a/test/src/module/command/backupTest.c b/test/src/module/command/backupTest.c index 6e77c6caa..3bec7d9b8 100644 --- a/test/src/module/command/backupTest.c +++ b/test/src/module/command/backupTest.c @@ -1090,21 +1090,38 @@ testRun(void) { TEST_TITLE("block incremental config map"); + // Configure repo2 rather than repo1 so the index in the option name must come from the repo key + StringList *argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "test1"); + hrnCfgArgKeyRawZ(argList, cfgOptRepoPath, 2, TEST_PATH "/repo"); + hrnCfgArgKeyRawZ(argList, cfgOptRepoRetentionFull, 2, "1"); + hrnCfgArgRawZ(argList, cfgOptPgPath, TEST_PATH "/pg1"); + HRN_CFG_LOAD(cfgCmdBackup, argList); + TEST_ERROR( backupBlockIncrMapSize(cfgOptRepoBlockSizeMap, 0, STRDEF("0")), OptionInvalidValueError, - "'0' is not valid for 'repo1-block-size-map' option"); + "'0' is not valid for 'repo2-block-size-map' option"); TEST_ERROR( backupBlockIncrMapSize(cfgOptRepoBlockSizeMap, 0, STRDEF("Z")), OptionInvalidValueError, - "'Z' is not valid for 'repo1-block-size-map' option"); + "'Z' is not valid for 'repo2-block-size-map' option"); TEST_ERROR( backupBlockIncrMapSize(cfgOptRepoBlockSizeMap, 0, STRDEF("5GiB")), OptionInvalidValueError, - "'5GiB' is not valid for 'repo1-block-size-map' option"); + "'5GiB' is not valid for 'repo2-block-size-map' option"); TEST_ERROR( backupBlockIncrMapChecksumSize(cfgOptRepoBlockChecksumSizeMap, 0, VARSTRDEF("Z")), OptionInvalidValueError, - "'Z' is not valid for 'repo1-block-checksum-size-map' option"); + "'Z' is not valid for 'repo2-block-checksum-size-map' option"); TEST_ERROR( backupBlockIncrMapChecksumSize(cfgOptRepoBlockChecksumSizeMap, 0, VARSTRDEF("5")), OptionInvalidValueError, - "'5' is not valid for 'repo1-block-checksum-size-map' option"); + "'5' is not valid for 'repo2-block-checksum-size-map' option"); + + // A key of the checksum size map is reported against the checksum size map rather than the size map + hrnCfgArgKeyRawBool(argList, cfgOptRepoBundle, 2, true); + hrnCfgArgKeyRawBool(argList, cfgOptRepoBlock, 2, true); + hrnCfgArgKeyRawZ(argList, cfgOptRepoBlockChecksumSizeMap, 2, "Z=16"); + HRN_CFG_LOAD(cfgCmdBackup, argList); + + TEST_ERROR( + backupBlockIncrMap(), OptionInvalidValueError, "'Z' is not valid for 'repo2-block-checksum-size-map' option"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("full backup with zero block");