Skip to content
Permalink
Browse files

storageList() returns an empty list by default for missing paths.

The prior behavior was to return NULL so the caller would know the path was missing, but this is rarely useful, complicates the calling code, and increases the chance of segfaults.

The .nullOnMissing param has been added to enable the prior behavior.
  • Loading branch information...
dwsteele committed May 24, 2019
1 parent 707e4a2 commit 96770c529be4df408b9d1fc39fc5157e90555375
@@ -317,7 +317,7 @@ walSegmentFind(const Storage *storage, const String *archiveId, const String *wa
StringList *list = storageListP(
storage, strNewFmt(STORAGE_REPO_ARCHIVE "/%s/%s", strPtr(archiveId), strPtr(strSubN(walSegment, 0, 16))),
.expression = strNewFmt("^%s%s-[0-f]{40}(\\.gz){0,1}$", strPtr(strSubN(walSegment, 0, 24)),
walIsPartial(walSegment) ? WAL_SEGMENT_PARTIAL_EXT : ""));
walIsPartial(walSegment) ? WAL_SEGMENT_PARTIAL_EXT : ""), .nullOnMissing = true);

// If there are results
if (list != NULL && strLstSize(list) > 0)
@@ -185,7 +185,7 @@ cmdArchiveGet(void)

// Get a list of WAL segments left in the queue
StringList *queue = storageListP(
storageSpool(), STORAGE_SPOOL_ARCHIVE_IN_STR, .expression = WAL_SEGMENT_REGEXP_STR);
storageSpool(), STORAGE_SPOOL_ARCHIVE_IN_STR, .expression = WAL_SEGMENT_REGEXP_STR, .errorOnMissing = true);

if (strLstSize(queue) > 0)
{
@@ -124,17 +124,13 @@ archiveDbList(const String *stanza, const InfoPgData *pgData, VariantList *archi
Variant *archiveInfo = varNewKv(kvNew());

// Get a list of WAL directories in the archive repo from oldest to newest, if any exist
StringList *walDir = storageListP(storageRepo(), archivePath, .expression = WAL_SEGMENT_DIR_REGEXP_STR);
StringList *walDir = strLstSort(
storageListP(storageRepo(), archivePath, .expression = WAL_SEGMENT_DIR_REGEXP_STR), sortOrderAsc);

if (walDir != NULL)
if (strLstSize(walDir) > 0)
{
unsigned int sizeWalDir = strLstSize(walDir);

if (sizeWalDir > 1)
walDir = strLstSort(walDir, sortOrderAsc);

// Not every WAL dir has WAL files so check each
for (unsigned int idx = 0; idx < sizeWalDir; idx++)
for (unsigned int idx = 0; idx < strLstSize(walDir); idx++)
{
// Get a list of all WAL in this WAL dir
StringList *list = storageListP(
@@ -152,7 +148,7 @@ archiveDbList(const String *stanza, const InfoPgData *pgData, VariantList *archi
}

// Iterate through the directory list in the reverse so processing newest first. Cast comparison to an int for readability.
for (unsigned int idx = sizeWalDir - 1; (int)idx >= 0; idx--)
for (unsigned int idx = strLstSize(walDir) - 1; (int)idx >= 0; idx--)
{
// Get a list of all WAL in this WAL dir
StringList *list = storageListP(
@@ -551,12 +547,12 @@ infoRender(void)
const String *stanza = cfgOptionTest(cfgOptStanza) ? cfgOptionStr(cfgOptStanza) : NULL;

// Get a list of stanzas in the backup directory.
StringList *stanzaList = storageListP(storageRepo(), STORAGE_PATH_BACKUP_STR, .errorOnMissing = false);
StringList *stanzaList = storageListNP(storageRepo(), STORAGE_PATH_BACKUP_STR);
VariantList *infoList = varLstNew();
String *resultStr = strNew("");

// If the backup storage exists, then search for and process any stanzas
if (stanzaList != NULL && strLstSize(stanzaList) > 0)
if (strLstSize(stanzaList) > 0)
infoList = stanzaInfoList(stanza, stanzaList);

// Format text output
@@ -379,7 +379,8 @@ cfgFileLoad( // NOTE: Pas

// Get a list of conf files from the specified path -error on missing directory if the option was passed on the command line
StringList *list = storageListP(
storageLocal(), configIncludePath, .expression = STRDEF(".+\\.conf$"), .errorOnMissing = configIncludeRequired);
storageLocal(), configIncludePath, .expression = STRDEF(".+\\.conf$"), .errorOnMissing = configIncludeRequired,
.nullOnMissing = !configIncludeRequired);

// If conf files are found, then add them to the config string
if (list != NULL && strLstSize(list) > 0)
@@ -310,10 +310,12 @@ storageList(const Storage *this, const String *pathExp, StorageListParam param)
FUNCTION_LOG_PARAM(STORAGE, this);
FUNCTION_LOG_PARAM(STRING, pathExp);
FUNCTION_LOG_PARAM(BOOL, param.errorOnMissing);
FUNCTION_LOG_PARAM(BOOL, param.nullOnMissing);
FUNCTION_LOG_PARAM(STRING, param.expression);
FUNCTION_LOG_END();

ASSERT(this != NULL);
ASSERT(!param.errorOnMissing || !param.nullOnMissing);

StringList *result = NULL;

@@ -322,8 +324,16 @@ storageList(const Storage *this, const String *pathExp, StorageListParam param)
// Build the path
String *path = storagePathNP(this, pathExp);

// Get the list
result = this->interface.list(this->driver, path, param.errorOnMissing, param.expression);

// Build an empty list if the directory does not exist by default. This makes the logic in calling functions simpler
// when they don't care if the path is missing.
if (result == NULL && !param.nullOnMissing)
result = strLstNew();

// Move list up to the old context
result = strLstMove(this->interface.list(this->driver, path, param.errorOnMissing, param.expression), MEM_CONTEXT_OLD());
result = strLstMove(result, MEM_CONTEXT_OLD());
}
MEM_CONTEXT_TEMP_END();

@@ -100,6 +100,7 @@ storageList
typedef struct StorageListParam
{
bool errorOnMissing;
bool nullOnMissing;
const String *expression;
} StorageListParam;

@@ -344,7 +344,8 @@ testRun(void)
storageListP(storageTest, strNew(BOGUS_STR), .errorOnMissing = true), PathOpenError,
"unable to open path '%s/BOGUS' for read: [2] No such file or directory", testPath());

TEST_RESULT_PTR(storageListNP(storageTest, strNew(BOGUS_STR)), NULL, "ignore missing dir");
TEST_RESULT_PTR(storageListP(storageTest, strNew(BOGUS_STR), .nullOnMissing = true), NULL, "null for missing dir");
TEST_RESULT_UINT(strLstSize(storageListNP(storageTest, strNew(BOGUS_STR))), 0, "empty list for missing dir");

// -------------------------------------------------------------------------------------------------------------------------
TEST_ERROR_FMT(
@@ -72,8 +72,8 @@ testRun(void)
TEST_ASSIGN(storageRemote, storageRepoGet(strNew(STORAGE_TYPE_POSIX), false), "get remote repo storage");
storagePathCreateNP(storageTest, strNew("repo"));

TEST_RESULT_STR(strPtr(strLstJoin(storageListNP(storageRemote, NULL), ",")), "" , "list empty path");
TEST_RESULT_PTR(storageListNP(storageRemote, strNew(BOGUS_STR)), NULL , "missing directory ignored");
TEST_RESULT_PTR(storageListP(storageRemote, strNew(BOGUS_STR), .nullOnMissing = true), NULL, "null for missing dir");
TEST_RESULT_UINT(strLstSize(storageListNP(storageRemote, NULL)), 0, "empty list for missing dir");

// -------------------------------------------------------------------------------------------------------------------------
storagePathCreateNP(storageTest, strNew("repo/testy"));

0 comments on commit 96770c5

Please sign in to comment.
You can’t perform that action at this time.