db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-14 10:13:05 +02:00
Code Issues Releases Activity

Don't use negations in objects below Storage.

Browse Source 
The Storage object represents some some optional parameters as negated if the default is true.  This allows sensible defaults without having to specify most optional parameters.

However, there's no need to propagate this down to functions that require all parameters to be passed -- it makes the code and logging more confusing.  Rename the parameters and update logic to remove negations.
This commit is contained in:
David Steele 2018-09-13 17:53:48 -04:00
parent 1fb9fe7026
commit ab1762663c
5 changed files with 27 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/xml/release.xml
View File

@ -53,7 +53,7 @@
</release-item> </release-item>
<release-item> <release-item>
<p>Posix file functions now differentiate between open and missing errors.</p> <p>Storage refactoring. Posix file functions now differentiate between open and missing errors. Don't use negations in objects below Storage.</p>
</release-item> </release-item>
<release-item> <release-item>

44
src/storage/driver/posix/driverWrite.c
View File

@ -26,10 +26,10 @@ struct StorageFileWritePosix
String *nameTmp; String *nameTmp;
mode_t modeFile; mode_t modeFile;
mode_t modePath; mode_t modePath;
bool noCreatePath; bool createPath;
bool noSyncFile; bool syncFile;
bool noSyncPath; bool syncPath;
bool noAtomic; bool atomic;
int handle; int handle;
}; };
@ -47,16 +47,16 @@ Create a new file
***********************************************************************************************************************************/ ***********************************************************************************************************************************/
StorageFileWritePosix * StorageFileWritePosix *
storageFileWritePosixNew( storageFileWritePosixNew(
const String *name, mode_t modeFile, mode_t modePath, bool noCreatePath, bool noSyncFile, bool noSyncPath, bool noAtomic) const String *name, mode_t modeFile, mode_t modePath, bool createPath, bool syncFile, bool syncPath, bool atomic)
{ {
FUNCTION_DEBUG_BEGIN(logLevelTrace); FUNCTION_DEBUG_BEGIN(logLevelTrace);
FUNCTION_DEBUG_PARAM(STRING, name); FUNCTION_DEBUG_PARAM(STRING, name);
FUNCTION_DEBUG_PARAM(MODE, modeFile); FUNCTION_DEBUG_PARAM(MODE, modeFile);
FUNCTION_DEBUG_PARAM(MODE, modePath); FUNCTION_DEBUG_PARAM(MODE, modePath);
FUNCTION_DEBUG_PARAM(BOOL, noCreatePath); FUNCTION_DEBUG_PARAM(BOOL, createPath);
FUNCTION_DEBUG_PARAM(BOOL, noSyncFile); FUNCTION_DEBUG_PARAM(BOOL, syncFile);
FUNCTION_DEBUG_PARAM(BOOL, noSyncPath); FUNCTION_DEBUG_PARAM(BOOL, syncPath);
FUNCTION_DEBUG_PARAM(BOOL, noAtomic); FUNCTION_DEBUG_PARAM(BOOL, atomic);
FUNCTION_TEST_ASSERT(name != NULL); FUNCTION_TEST_ASSERT(name != NULL);
FUNCTION_DEBUG_END(); FUNCTION_DEBUG_END();
@ -72,13 +72,13 @@ storageFileWritePosixNew(
this->memContext = MEM_CONTEXT_NEW(); this->memContext = MEM_CONTEXT_NEW();
this->path = strPath(name); this->path = strPath(name);
this->name = strDup(name); this->name = strDup(name);
this->nameTmp = noAtomic ? this->name : strNewFmt("%s." STORAGE_FILE_TEMP_EXT, strPtr(name)); this->nameTmp = atomic ? strNewFmt("%s." STORAGE_FILE_TEMP_EXT, strPtr(name)) : this->name;
this->modeFile = modeFile; this->modeFile = modeFile;
this->modePath = modePath; this->modePath = modePath;
this->noCreatePath = noCreatePath; this->createPath = createPath;
this->noSyncFile = noSyncFile; this->syncFile = syncFile;
this->noSyncPath = noSyncPath; this->syncPath = syncPath;
this->noAtomic = noAtomic; this->atomic = atomic;
this->handle = -1; this->handle = -1;
} }
@ -102,7 +102,7 @@ storageFileWritePosixOpen(StorageFileWritePosix *this)
// Open the file and handle errors // Open the file and handle errors
this->handle = storageFilePosixOpen( this->handle = storageFilePosixOpen(
this->nameTmp, FILE_OPEN_FLAGS, this->modeFile, !this->noCreatePath, true, FILE_OPEN_PURPOSE); this->nameTmp, FILE_OPEN_FLAGS, this->modeFile, this->createPath, true, FILE_OPEN_PURPOSE);
// If path is missing // If path is missing
if (this->handle == -1) if (this->handle == -1)
@ -159,21 +159,21 @@ storageFileWritePosixClose(StorageFileWritePosix *this)
if (this->handle != -1) if (this->handle != -1)
{ {
// Sync the file // Sync the file
if (!this->noSyncFile) if (this->syncFile)
storageFilePosixSync(this->handle, this->name, true, false); storageFilePosixSync(this->handle, this->name, true, false);
// Close the file // Close the file
storageFilePosixClose(this->handle, this->name, true); storageFilePosixClose(this->handle, this->name, true);
// Rename from temp file // Rename from temp file
if (!this->noAtomic) if (this->atomic)
{ {
if (rename(strPtr(this->nameTmp), strPtr(this->name)) == -1) if (rename(strPtr(this->nameTmp), strPtr(this->name)) == -1)
THROW_SYS_ERROR_FMT(FileMoveError, "unable to move '%s' to '%s'", strPtr(this->nameTmp), strPtr(this->name)); THROW_SYS_ERROR_FMT(FileMoveError, "unable to move '%s' to '%s'", strPtr(this->nameTmp), strPtr(this->name));
} }
// Sync the path // Sync the path
if (!this->noSyncPath) if (this->syncPath)
storageDriverPosixPathSync(this->path, false); storageDriverPosixPathSync(this->path, false);
// This marks the file as closed // This marks the file as closed
@ -197,7 +197,7 @@ storageFileWritePosixAtomic(StorageFileWritePosix *this)
FUNCTION_TEST_ASSERT(this != NULL); FUNCTION_TEST_ASSERT(this != NULL);
FUNCTION_TEST_END(); FUNCTION_TEST_END();
FUNCTION_TEST_RESULT(BOOL, !this->noAtomic); FUNCTION_TEST_RESULT(BOOL, this->atomic);
} }
/*********************************************************************************************************************************** /***********************************************************************************************************************************
@ -212,7 +212,7 @@ storageFileWritePosixCreatePath(StorageFileWritePosix *this)
FUNCTION_TEST_ASSERT(this != NULL); FUNCTION_TEST_ASSERT(this != NULL);
FUNCTION_TEST_END(); FUNCTION_TEST_END();
FUNCTION_TEST_RESULT(BOOL, !this->noCreatePath); FUNCTION_TEST_RESULT(BOOL, this->createPath);
} }
/*********************************************************************************************************************************** /***********************************************************************************************************************************
@ -287,7 +287,7 @@ storageFileWritePosixSyncFile(StorageFileWritePosix *this)
FUNCTION_TEST_ASSERT(this != NULL); FUNCTION_TEST_ASSERT(this != NULL);
FUNCTION_TEST_END(); FUNCTION_TEST_END();
FUNCTION_TEST_RESULT(BOOL, !this->noSyncFile); FUNCTION_TEST_RESULT(BOOL, this->syncFile);
} }
/*********************************************************************************************************************************** /***********************************************************************************************************************************
@ -302,7 +302,7 @@ storageFileWritePosixSyncPath(StorageFileWritePosix *this)
FUNCTION_TEST_ASSERT(this != NULL); FUNCTION_TEST_ASSERT(this != NULL);
FUNCTION_TEST_END(); FUNCTION_TEST_END();
FUNCTION_TEST_RESULT(BOOL, !this->noSyncPath); FUNCTION_TEST_RESULT(BOOL, this->syncPath);
} }
/*********************************************************************************************************************************** /***********************************************************************************************************************************

2
src/storage/driver/posix/driverWrite.h
View File

@ -18,7 +18,7 @@ typedef struct StorageFileWritePosix StorageFileWritePosix;
Constructor Constructor
***********************************************************************************************************************************/ ***********************************************************************************************************************************/
StorageFileWritePosix *storageFileWritePosixNew( StorageFileWritePosix *storageFileWritePosixNew(
const String *name, mode_t modeFile, mode_t modePath, bool noCreatePath, bool noSyncFile, bool noSyncPath, bool noAtomic); const String *name, mode_t modeFile, mode_t modePath, bool createPath, bool syncFile, bool syncPath, bool atomic);
/*********************************************************************************************************************************** /***********************************************************************************************************************************
Functions Functions

4
src/storage/storage.c
View File

@ -362,8 +362,8 @@ storageNewWrite(const Storage *this, const String *fileExp, StorageNewWriteParam
{ {
result = storageFileWriteNew( result = storageFileWriteNew(
storagePathNP(this, fileExp), param.modeFile != 0 ? param.modeFile : this->modeFile, storagePathNP(this, fileExp), param.modeFile != 0 ? param.modeFile : this->modeFile,
param.modePath != 0 ? param.modePath : this->modePath, param.noCreatePath, param.noSyncFile, param.noSyncPath, param.modePath != 0 ? param.modePath : this->modePath, !param.noCreatePath, !param.noSyncFile, !param.noSyncPath,
param.noAtomic); !param.noAtomic);
if (param.filterGroup != NULL) if (param.filterGroup != NULL)
ioWriteFilterGroupSet(storageFileWriteIo(result), param.filterGroup); ioWriteFilterGroupSet(storageFileWriteIo(result), param.filterGroup);

2
test/src/module/storage/fileTest.c
View File

@ -224,7 +224,7 @@ testRun(void)
"unable to sync '%s': [9] Bad file descriptor", strPtr(fileName)); "unable to sync '%s': [9] Bad file descriptor", strPtr(fileName));
// Disable file sync so the close can be reached // Disable file sync so the close can be reached
file->fileDriver->noSyncFile = true; file->fileDriver->syncFile = false;
TEST_ERROR_FMT( TEST_ERROR_FMT(
storageFileWritePosixClose(storageFileWriteFileDriver(file)), FileCloseError, storageFileWritePosixClose(storageFileWriteFileDriver(file)), FileCloseError,