1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00

Add strLstNewFmt().

Simplifies adding a formatted string to a list and removes a common cause of leaks.
This commit is contained in:
David Steele 2022-04-25 11:47:43 -04:00
parent 3475514b61
commit 7900660d3a
17 changed files with 78 additions and 60 deletions

View File

@ -89,11 +89,9 @@ ArchiveGetFileResult archiveGetFile(
{ {
MEM_CONTEXT_PRIOR_BEGIN() MEM_CONTEXT_PRIOR_BEGIN()
{ {
strLstAdd( strLstAddFmt(
result.warnList, result.warnList, "%s: %s [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, actual->repoIdx), strZ(actual->file),
strNewFmt( errorTypeName(errorType()), errorMessage());
"%s: %s [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, actual->repoIdx), strZ(actual->file),
errorTypeName(errorType()), errorMessage()));
} }
MEM_CONTEXT_PRIOR_END(); MEM_CONTEXT_PRIOR_END();
} }

View File

@ -38,9 +38,7 @@ archivePushErrorAdd(StringList *errorList, unsigned int repoIdx)
FUNCTION_TEST_PARAM(UINT, repoIdx); FUNCTION_TEST_PARAM(UINT, repoIdx);
FUNCTION_TEST_END(); FUNCTION_TEST_END();
strLstAdd( strLstAddFmt(errorList, "%s: [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, repoIdx), errorTypeName(errorType()), errorMessage());
errorList,
strNewFmt("%s: [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, repoIdx), errorTypeName(errorType()), errorMessage()));
FUNCTION_TEST_RETURN_VOID(); FUNCTION_TEST_RETURN_VOID();
} }
@ -201,12 +199,11 @@ archivePushFile(
MEM_CONTEXT_PRIOR_BEGIN() MEM_CONTEXT_PRIOR_BEGIN()
{ {
// Add warning to the result that will be returned to the main process // Add warning to the result that will be returned to the main process
strLstAdd( strLstAddFmt(
result.warnList, result.warnList,
strNewFmt(
"WAL file '%s' already exists in the %s archive with the same checksum" "WAL file '%s' already exists in the %s archive with the same checksum"
"\nHINT: this is valid in some recovery scenarios but may also indicate a problem.", "\nHINT: this is valid in some recovery scenarios but may also indicate a problem.",
strZ(archiveFile), cfgOptionGroupName(cfgOptGrpRepo, repoData->repoIdx))); strZ(archiveFile), cfgOptionGroupName(cfgOptGrpRepo, repoData->repoIdx));
} }
MEM_CONTEXT_PRIOR_END(); MEM_CONTEXT_PRIOR_END();
} }

View File

@ -276,10 +276,9 @@ archivePushCheck(bool pgPathSet)
} }
CATCH_ANY() CATCH_ANY()
{ {
strLstAdd( strLstAddFmt(
result.errorList, result.errorList, "%s: [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, repoIdx), errorTypeName(errorType()),
strNewFmt( errorMessage());
"%s: [%s] %s", cfgOptionGroupName(cfgOptGrpRepo, repoIdx), errorTypeName(errorType()), errorMessage()));
} }
TRY_END(); TRY_END();
} }

View File

@ -1488,7 +1488,7 @@ backupProcessQueue(const BackupData *const backupData, Manifest *const manifest,
const ManifestTarget *target = manifestTarget(manifest, targetIdx); const ManifestTarget *target = manifestTarget(manifest, targetIdx);
if (target->tablespaceId != 0) if (target->tablespaceId != 0)
strLstAdd(targetList, strNewFmt("%s/", strZ(target->name))); strLstAddFmt(targetList, "%s/", strZ(target->name));
} }
// Generate the processing queues (there is always at least one) // Generate the processing queues (there is always at least one)

View File

@ -114,11 +114,9 @@ cmdOption(void)
for (unsigned int keyIdx = 0; keyIdx < varLstSize(keyList); keyIdx++) for (unsigned int keyIdx = 0; keyIdx < varLstSize(keyList); keyIdx++)
{ {
strLstAdd( strLstAddFmt(
valueList, valueList, "%s=%s", strZ(varStr(varLstGet(keyList, keyIdx))),
strNewFmt( strZ(varStrForce(kvGet(optionKv, varLstGet(keyList, keyIdx)))));
"%s=%s", strZ(varStr(varLstGet(keyList, keyIdx))),
strZ(varStrForce(kvGet(optionKv, varLstGet(keyList, keyIdx))))));
} }
} }
// Generate values for list options // Generate values for list options

View File

@ -2010,7 +2010,7 @@ restoreProcessQueue(Manifest *manifest, List **queueList)
const ManifestTarget *target = manifestTarget(manifest, targetIdx); const ManifestTarget *target = manifestTarget(manifest, targetIdx);
if (target->tablespaceId != 0) if (target->tablespaceId != 0)
strLstAdd(targetList, strNewFmt("%s/", strZ(target->name))); strLstAddFmt(targetList, "%s/", strZ(target->name));
} }
// Generate the processing queues // Generate the processing queues

View File

@ -151,6 +151,39 @@ strLstAdd(StringList *this, const String *string)
FUNCTION_TEST_RETURN(STRING, result); FUNCTION_TEST_RETURN(STRING, result);
} }
String *
strLstAddFmt(StringList *const this, const char *const format, ...)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(STRING_LIST, this);
FUNCTION_TEST_PARAM(STRINGZ, format);
FUNCTION_TEST_END();
// Determine how long the allocated string needs to be
va_list argumentList;
va_start(argumentList, format);
const size_t size = (size_t)vsnprintf(NULL, 0, format, argumentList);
va_end(argumentList);
// Format string
va_start(argumentList, format);
char *const buffer = memNew(size + 1);
vsnprintf(buffer, size + 1, format, argumentList);
va_end(argumentList);
String *result = NULL;
MEM_CONTEXT_BEGIN(lstMemContext((List *)this))
{
result = strLstAddInternal(this, strNewZN(buffer, size));
}
MEM_CONTEXT_END();
memFree(buffer);
FUNCTION_TEST_RETURN(STRING, result);
}
String * String *
strLstAddIfMissing(StringList *this, const String *string) strLstAddIfMissing(StringList *this, const String *string)
{ {

View File

@ -68,6 +68,7 @@ Functions
***********************************************************************************************************************************/ ***********************************************************************************************************************************/
// Add String to the list // Add String to the list
String *strLstAdd(StringList *this, const String *string); String *strLstAdd(StringList *this, const String *string);
String *strLstAddFmt(StringList *this, const char *format, ...) __attribute__((format(printf, 2, 3)));
String *strLstAddZ(StringList *this, const char *string); String *strLstAddZ(StringList *this, const char *string);
String *strLstAddIfMissing(StringList *this, const String *string); String *strLstAddIfMissing(StringList *this, const String *string);

View File

@ -77,15 +77,13 @@ cfgExecParam(ConfigCommand commandId, ConfigCommandRole commandRoleId, const Key
// If the option was reset // If the option was reset
if (value == NULL && cfgOptionValid(optionId) && cfgOptionIdxReset(optionId, optionIdx)) if (value == NULL && cfgOptionValid(optionId) && cfgOptionIdxReset(optionId, optionIdx))
{ {
strLstAdd(result, strNewFmt("--reset-%s", cfgOptionIdxName(optionId, optionIdx))); strLstAddFmt(result, "--reset-%s", cfgOptionIdxName(optionId, optionIdx));
} }
// Else format the value if found, even if the option is not valid for the command // Else format the value if found, even if the option is not valid for the command
else if (value != NULL && (!local || exists || cfgOptionIdxSource(optionId, optionIdx) == cfgSourceParam)) else if (value != NULL && (!local || exists || cfgOptionIdxSource(optionId, optionIdx) == cfgSourceParam))
{ {
if (varType(value) == varTypeBool) if (varType(value) == varTypeBool)
{ strLstAddFmt(result, "--%s%s", varBool(value) ? "" : "no-", cfgOptionIdxName(optionId, optionIdx));
strLstAdd(result, strNewFmt("--%s%s", varBool(value) ? "" : "no-", cfgOptionIdxName(optionId, optionIdx)));
}
else else
{ {
StringList *valueList = NULL; StringList *valueList = NULL;
@ -99,11 +97,9 @@ cfgExecParam(ConfigCommand commandId, ConfigCommandRole commandRoleId, const Key
for (unsigned int keyIdx = 0; keyIdx < varLstSize(keyList); keyIdx++) for (unsigned int keyIdx = 0; keyIdx < varLstSize(keyList); keyIdx++)
{ {
strLstAdd( strLstAddFmt(
valueList, valueList, "%s=%s", strZ(varStr(varLstGet(keyList, keyIdx))),
strNewFmt( strZ(varStrForce(kvGet(optionKv, varLstGet(keyList, keyIdx)))));
"%s=%s", strZ(varStr(varLstGet(keyList, keyIdx))),
strZ(varStrForce(kvGet(optionKv, varLstGet(keyList, keyIdx))))));
} }
} }
else if (varType(value) == varTypeVariantList) else if (varType(value) == varTypeVariantList)
@ -129,7 +125,7 @@ cfgExecParam(ConfigCommand commandId, ConfigCommandRole commandRoleId, const Key
if (quote && strchr(strZ(value), ' ') != NULL) if (quote && strchr(strZ(value), ' ') != NULL)
value = strNewFmt("\"%s\"", strZ(value)); value = strNewFmt("\"%s\"", strZ(value));
strLstAdd(result, strNewFmt("--%s=%s", cfgOptionIdxName(optionId, optionIdx), strZ(value))); strLstAddFmt(result, "--%s=%s", cfgOptionIdxName(optionId, optionIdx), strZ(value));
} }
} }
} }

View File

@ -1748,11 +1748,11 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis
if (stanza != NULL) if (stanza != NULL)
{ {
strLstAdd(sectionList, strNewFmt("%s:%s", strZ(stanza), cfgParseCommandName(config->command))); strLstAddFmt(sectionList, "%s:%s", strZ(stanza), cfgParseCommandName(config->command));
strLstAdd(sectionList, stanza); strLstAdd(sectionList, stanza);
} }
strLstAdd(sectionList, strNewFmt(CFGDEF_SECTION_GLOBAL ":%s", cfgParseCommandName(config->command))); strLstAddFmt(sectionList, CFGDEF_SECTION_GLOBAL ":%s", cfgParseCommandName(config->command));
strLstAddZ(sectionList, CFGDEF_SECTION_GLOBAL); strLstAddZ(sectionList, CFGDEF_SECTION_GLOBAL);
// Loop through sections to search for options // Loop through sections to search for options

View File

@ -572,7 +572,7 @@ pgLsnRangeToWalSegmentList(
unsigned int minorPerMajor = 0xFFFFFFFF / walSegmentSize; unsigned int minorPerMajor = 0xFFFFFFFF / walSegmentSize;
// Create list // Create list
strLstAdd(result, strNewFmt("%08X%08X%08X", timeline, startMajor, startMinor)); strLstAddFmt(result, "%08X%08X%08X", timeline, startMajor, startMinor);
while (!(startMajor == stopMajor && startMinor == stopMinor)) while (!(startMajor == stopMajor && startMinor == stopMinor))
{ {
@ -584,7 +584,7 @@ pgLsnRangeToWalSegmentList(
startMinor = 0; startMinor = 0;
} }
strLstAdd(result, strNewFmt("%08X%08X%08X", timeline, startMajor, startMinor)); strLstAddFmt(result, "%08X%08X%08X", timeline, startMajor, startMinor);
} }
strLstMove(result, memContextPrior()); strLstMove(result, memContextPrior());

View File

@ -597,15 +597,13 @@ protocolRemoteParamSsh(const ProtocolStorageType protocolStorageType, const unsi
if (cfgOptionIdxTest(optHostPort, hostIdx)) if (cfgOptionIdxTest(optHostPort, hostIdx))
{ {
strLstAddZ(result, "-p"); strLstAddZ(result, "-p");
strLstAdd(result, strNewFmt("%u", cfgOptionIdxUInt(optHostPort, hostIdx))); strLstAddFmt(result, "%u", cfgOptionIdxUInt(optHostPort, hostIdx));
} }
// Append user/host // Append user/host
strLstAdd( strLstAddFmt(
result, result, "%s@%s", strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHostUser : cfgOptPgHostUser, hostIdx)),
strNewFmt( strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHost : cfgOptPgHost, hostIdx)));
"%s@%s", strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHostUser : cfgOptPgHostUser, hostIdx)),
strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHost : cfgOptPgHost, hostIdx))));
// Add remote command and parameters // Add remote command and parameters
StringList *paramList = protocolRemoteParam(protocolStorageType, hostIdx); StringList *paramList = protocolRemoteParam(protocolStorageType, hostIdx);

View File

@ -144,7 +144,7 @@ hrnCfgArgRawZ(StringList *argList, ConfigOption optionId, const char *value)
void void
hrnCfgArgKeyRawZ(StringList *argList, ConfigOption optionId, unsigned optionKey, const char *value) hrnCfgArgKeyRawZ(StringList *argList, ConfigOption optionId, unsigned optionKey, const char *value)
{ {
strLstAdd(argList, strNewFmt("--%s=%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1), value)); strLstAddFmt(argList, "--%s=%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1), value);
} }
void void
@ -171,7 +171,7 @@ hrnCfgArgRawBool(StringList *argList, ConfigOption optionId, bool value)
void void
hrnCfgArgKeyRawBool(StringList *argList, ConfigOption optionId, unsigned optionKey, bool value) hrnCfgArgKeyRawBool(StringList *argList, ConfigOption optionId, unsigned optionKey, bool value)
{ {
strLstAdd(argList, strNewFmt("--%s%s", value ? "" : "no-", cfgParseOptionKeyIdxName(optionId, optionKey - 1))); strLstAddFmt(argList, "--%s%s", value ? "" : "no-", cfgParseOptionKeyIdxName(optionId, optionKey - 1));
} }
void void
@ -183,7 +183,7 @@ hrnCfgArgRawNegate(StringList *argList, ConfigOption optionId)
void void
hrnCfgArgKeyRawNegate(StringList *argList, ConfigOption optionId, unsigned optionKey) hrnCfgArgKeyRawNegate(StringList *argList, ConfigOption optionId, unsigned optionKey)
{ {
strLstAdd(argList, strNewFmt("--no-%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1))); strLstAddFmt(argList, "--no-%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1));
} }
void void
@ -195,7 +195,7 @@ hrnCfgArgRawReset(StringList *argList, ConfigOption optionId)
void void
hrnCfgArgKeyRawReset(StringList *argList, ConfigOption optionId, unsigned optionKey) hrnCfgArgKeyRawReset(StringList *argList, ConfigOption optionId, unsigned optionKey)
{ {
strLstAdd(argList, strNewFmt("--reset-%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1))); strLstAddFmt(argList, "--reset-%s", cfgParseOptionKeyIdxName(optionId, optionKey - 1));
} }
/**********************************************************************************************************************************/ /**********************************************************************************************************************************/

View File

@ -324,15 +324,15 @@ hrnStorageList(const Storage *const storage, const char *const path, const char
break; break;
case storageTypeLink: case storageTypeLink:
strLstAdd(listStr, strNewFmt("%s>", strZ(info->name))); strLstAddFmt(listStr, "%s>", strZ(info->name));
break; break;
case storageTypePath: case storageTypePath:
strLstAdd(listStr, strNewFmt("%s/", strZ(info->name))); strLstAddFmt(listStr, "%s/", strZ(info->name));
break; break;
case storageTypeSpecial: case storageTypeSpecial:
strLstAdd(listStr, strNewFmt("%s*", strZ(info->name))); strLstAddFmt(listStr, "%s*", strZ(info->name));
break; break;
} }
} }

View File

@ -528,7 +528,7 @@ testRun(void)
argList = strLstNew(); argList = strLstNew();
hrnCfgArgRawZ(argList, cfgOptRepoPath, "/"); hrnCfgArgRawZ(argList, cfgOptRepoPath, "/");
strLstAdd(argList, strNewFmt(TEST_PATH "/repo/%s", strZ(fileName))); strLstAddFmt(argList, TEST_PATH "/repo/%s", strZ(fileName));
HRN_CFG_LOAD(cfgCmdRepoGet, argList); HRN_CFG_LOAD(cfgCmdRepoGet, argList);
writeBuffer = bufNew(0); writeBuffer = bufNew(0);
@ -701,11 +701,9 @@ testRun(void)
argList = strLstNew(); argList = strLstNew();
hrnCfgArgRawZ(argList, cfgOptRepoPath, TEST_PATH "/repo"); hrnCfgArgRawZ(argList, cfgOptRepoPath, TEST_PATH "/repo");
hrnCfgArgRawStrId(argList, cfgOptRepoCipherType, cipherTypeAes256Cbc); hrnCfgArgRawStrId(argList, cfgOptRepoCipherType, cipherTypeAes256Cbc);
strLstAdd( strLstAddFmt(
argList, argList, "%s/repo/" STORAGE_PATH_ARCHIVE "/test/12-1/000000010000000100000001-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
strNewFmt( TEST_PATH);
"%s/repo/" STORAGE_PATH_ARCHIVE "/test/12-1/000000010000000100000001-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
TEST_PATH));
HRN_CFG_LOAD(cfgCmdRepoGet, argList); HRN_CFG_LOAD(cfgCmdRepoGet, argList);
writeBuffer = bufNew(0); writeBuffer = bufNew(0);

View File

@ -341,7 +341,7 @@ testRun(void)
else else
{ {
TEST_RESULT_STR( TEST_RESULT_STR(
strLstAdd(list, strNewFmt("STR%02d", listIdx)), strNewFmt("STR%02d", listIdx), strLstAddFmt(list, "STR%02d", listIdx), strNewFmt("STR%02d", listIdx),
strZ(strNewFmt("add item %d", listIdx))); strZ(strNewFmt("add item %d", listIdx)));
} }
} }
@ -418,7 +418,7 @@ testRun(void)
if (listIdx == 0) if (listIdx == 0)
strLstAdd(list, NULL); strLstAdd(list, NULL);
else else
strLstAdd(list, strNewFmt("STR%02d", listIdx)); strLstAddFmt(list, "STR%02d", listIdx);
} }
// Check pointer // Check pointer

View File

@ -781,7 +781,7 @@ testRun(void)
argList = strLstNew(); argList = strLstNew();
strLstAddZ(argList, TEST_BACKREST_EXE); strLstAddZ(argList, TEST_BACKREST_EXE);
strLstAdd(argList, strNewFmt("--%s", optionMax)); strLstAddFmt(argList, "--%s", optionMax);
TEST_ERROR_FMT( TEST_ERROR_FMT(
configParse(storageTest, strLstSize(argList), strLstPtr(argList), false), OptionInvalidError, configParse(storageTest, strLstSize(argList), strLstPtr(argList), false), OptionInvalidError,
"option '%s' exceeds maximum size of 64", optionMax); "option '%s' exceeds maximum size of 64", optionMax);