1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2026-06-20 01:17:49 +02:00

Remove unused pathSync parameter in SFTP storage driver.

The SFTP storage driver did not set the pathSync method but had logic to see if pathSync was set. This led to some tortured tests since pathSync had to be injected into the interface after the storage object had been created.

Remove the pathSync parameter from storageWriteSftpNew() and also remove related tests.
This commit is contained in:
David Steele
2025-09-16 14:58:49 +02:00
parent 9c7f064864
commit 5cc9d25817
4 changed files with 4 additions and 127 deletions
+1 -2
View File
@@ -970,8 +970,7 @@ storageSftpNewWrite(THIS_VOID, const String *const file, const StorageInterfaceN
STORAGE_WRITE,
storageWriteSftpNew(
this, file, this->session, this->sftpSession, this->sftpHandle, param.modeFile, param.modePath, param.user, param.group,
param.timeModified, param.createPath, param.syncFile, this->interface.pathSync != NULL ? param.syncPath : false,
param.atomic, param.truncate));
param.timeModified, param.createPath, param.syncFile, param.atomic, param.truncate));
}
/**********************************************************************************************************************************/
+2 -4
View File
@@ -341,8 +341,8 @@ FN_EXTERN StorageWrite *
storageWriteSftpNew(
StorageSftp *const storage, const String *const name, LIBSSH2_SESSION *const session, LIBSSH2_SFTP *const sftpSession,
LIBSSH2_SFTP_HANDLE *const sftpHandle, const mode_t modeFile, const mode_t modePath, const String *const user,
const String *const group, const time_t timeModified, const bool createPath, const bool syncFile, const bool syncPath,
const bool atomic, const bool truncate)
const String *const group, const time_t timeModified, const bool createPath, const bool syncFile, const bool atomic,
const bool truncate)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_SFTP, storage);
@@ -357,7 +357,6 @@ storageWriteSftpNew(
FUNCTION_LOG_PARAM(TIME, timeModified);
FUNCTION_LOG_PARAM(BOOL, createPath);
FUNCTION_LOG_PARAM(BOOL, syncFile);
FUNCTION_LOG_PARAM(BOOL, syncPath);
FUNCTION_LOG_PARAM(BOOL, atomic);
FUNCTION_LOG_PARAM(BOOL, truncate);
FUNCTION_LOG_END();
@@ -387,7 +386,6 @@ storageWriteSftpNew(
.modeFile = modeFile,
.modePath = modePath,
.syncFile = syncFile,
.syncPath = syncPath,
.truncate = truncate,
.user = strDup(user),
.timeModified = timeModified,
+1 -1
View File
@@ -14,6 +14,6 @@ Constructors
FN_EXTERN StorageWrite *storageWriteSftpNew(
StorageSftp *storage, const String *name, LIBSSH2_SESSION *session, LIBSSH2_SFTP *sftpSession, LIBSSH2_SFTP_HANDLE *sftpHandle,
mode_t modeFile, mode_t modePath, const String *user, const String *group, time_t timeModified, bool createPath, bool syncFile,
bool syncPath, bool atomic, bool truncate);
bool atomic, bool truncate);
#endif
-120
View File
@@ -5435,126 +5435,6 @@ testRun(void)
memContextFree(objMemContext((StorageSftp *)storageDriver(storageTest)));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("write file - syncPath not NULL, .noSyncPath = true");
hrnLibSsh2ScriptSet((HrnLibSsh2 [])
{
HRNLIBSSH2_MACRO_STARTUP(),
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .resultNull = true, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_NO_SUCH_FILE},
{.function = HRNLIBSSH2_SFTP_MKDIR_EX, .param = "[\"" TEST_PATH "/sub1\",488]"},
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SFTP_FSYNC, .resultInt = LIBSSH2_ERROR_NONE},
{.function = HRNLIBSSH2_SFTP_CLOSE_HANDLE, .resultInt = LIBSSH2_ERROR_NONE},
HRNLIBSSH2_MACRO_SHUTDOWN()
});
storageTest = storageSftpNewP(
cfgOptionIdxStr(cfgOptRepoPath, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHost, repoIdx),
cfgOptionIdxUInt(cfgOptRepoSftpHostPort, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHostUser, repoIdx),
cfgOptionUInt64(cfgOptIoTimeout), cfgOptionIdxStr(cfgOptRepoSftpPrivateKeyFile, repoIdx),
cfgOptionIdxStrId(cfgOptRepoSftpHostKeyHashType, repoIdx), .modeFile = STORAGE_MODE_FILE_DEFAULT,
.modePath = STORAGE_MODE_PATH_DEFAULT, .keyPub = cfgOptionIdxStrNull(cfgOptRepoSftpPublicKeyFile, repoIdx),
.keyPassphrase = cfgOptionIdxStrNull(cfgOptRepoSftpPrivateKeyPassphrase, repoIdx),
.hostFingerprint = cfgOptionIdxStrNull(cfgOptRepoSftpHostFingerprint, repoIdx),
.hostKeyCheckType = cfgOptionIdxStrId(cfgOptRepoSftpHostKeyCheckType, repoIdx),
.knownHosts = strLstNewVarLst(cfgOptionIdxLst(cfgOptRepoSftpKnownHost, repoIdx)), .write = true);
// Make interface.pathSync != NULL
((StorageSftp *)storageDriver(storageTest))->interface.pathSync = malloc(1);
TEST_ASSIGN(
file, storageNewWriteP(storageTest, fileName, .noAtomic = true, .noSyncPath = true), "new write file (defaults)");
TEST_RESULT_VOID(ioWriteOpen(storageWriteIo(file)), "open file");
TEST_RESULT_INT(ioWriteFd(storageWriteIo(file)), -1, "check write fd");
TEST_RESULT_VOID(ioWriteClose(storageWriteIo(file)), "close file");
free(((StorageSftp *)storageDriver(storageTest))->interface.pathSync);
memContextFree(objMemContext((StorageSftp *)storageDriver(storageTest)));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("write file - syncPath not NULL, .noSyncPath = false");
hrnLibSsh2ScriptSet((HrnLibSsh2 [])
{
HRNLIBSSH2_MACRO_STARTUP(),
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .resultNull = true, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_NO_SUCH_FILE},
{.function = HRNLIBSSH2_SFTP_MKDIR_EX, .param = "[\"" TEST_PATH "/sub1\",488]"},
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SFTP_FSYNC, .resultInt = LIBSSH2_ERROR_NONE},
{.function = HRNLIBSSH2_SFTP_CLOSE_HANDLE, .resultInt = LIBSSH2_ERROR_NONE},
HRNLIBSSH2_MACRO_SHUTDOWN()
});
storageTest = storageSftpNewP(
cfgOptionIdxStr(cfgOptRepoPath, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHost, repoIdx),
cfgOptionIdxUInt(cfgOptRepoSftpHostPort, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHostUser, repoIdx),
cfgOptionUInt64(cfgOptIoTimeout), cfgOptionIdxStr(cfgOptRepoSftpPrivateKeyFile, repoIdx),
cfgOptionIdxStrId(cfgOptRepoSftpHostKeyHashType, repoIdx), .modeFile = STORAGE_MODE_FILE_DEFAULT,
.modePath = STORAGE_MODE_PATH_DEFAULT, .keyPub = cfgOptionIdxStrNull(cfgOptRepoSftpPublicKeyFile, repoIdx),
.keyPassphrase = cfgOptionIdxStrNull(cfgOptRepoSftpPrivateKeyPassphrase, repoIdx),
.hostFingerprint = cfgOptionIdxStrNull(cfgOptRepoSftpHostFingerprint, repoIdx),
.hostKeyCheckType = cfgOptionIdxStrId(cfgOptRepoSftpHostKeyCheckType, repoIdx),
.knownHosts = strLstNewVarLst(cfgOptionIdxLst(cfgOptRepoSftpKnownHost, repoIdx)), .write = true);
// Make interface.pathSync != NULL
((StorageSftp *)storageDriver(storageTest))->interface.pathSync = malloc(1);
TEST_ASSIGN(file, storageNewWriteP(storageTest, fileName, .noAtomic = true, .noSyncPath = false), "new write file");
TEST_RESULT_VOID(ioWriteOpen(storageWriteIo(file)), "open file");
TEST_RESULT_INT(ioWriteFd(storageWriteIo(file)), -1, "check write fd");
TEST_RESULT_VOID(ioWriteClose(storageWriteIo(file)), "close file");
free(((StorageSftp *)storageDriver(storageTest))->interface.pathSync);
memContextFree(objMemContext((StorageSftp *)storageDriver(storageTest)));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("write file - syncFile false");
hrnLibSsh2ScriptSet((HrnLibSsh2 [])
{
HRNLIBSSH2_MACRO_STARTUP(),
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .resultNull = true, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SESSION_LAST_ERRNO, .resultInt = LIBSSH2_ERROR_SFTP_PROTOCOL},
{.function = HRNLIBSSH2_SFTP_LAST_ERROR, .resultUInt = LIBSSH2_FX_NO_SUCH_FILE},
{.function = HRNLIBSSH2_SFTP_MKDIR_EX, .param = "[\"" TEST_PATH "/sub1\",488]"},
{.function = HRNLIBSSH2_SFTP_OPEN_EX, .param = "[\"" TEST_PATH "/sub1/testfile\",26,416,0]"},
{.function = HRNLIBSSH2_SFTP_CLOSE_HANDLE, .resultInt = LIBSSH2_ERROR_NONE},
HRNLIBSSH2_MACRO_SHUTDOWN()
});
storageTest = storageSftpNewP(
cfgOptionIdxStr(cfgOptRepoPath, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHost, repoIdx),
cfgOptionIdxUInt(cfgOptRepoSftpHostPort, repoIdx), cfgOptionIdxStr(cfgOptRepoSftpHostUser, repoIdx),
cfgOptionUInt64(cfgOptIoTimeout), cfgOptionIdxStr(cfgOptRepoSftpPrivateKeyFile, repoIdx),
cfgOptionIdxStrId(cfgOptRepoSftpHostKeyHashType, repoIdx), .modeFile = STORAGE_MODE_FILE_DEFAULT,
.modePath = STORAGE_MODE_PATH_DEFAULT, .keyPub = cfgOptionIdxStrNull(cfgOptRepoSftpPublicKeyFile, repoIdx),
.keyPassphrase = cfgOptionIdxStrNull(cfgOptRepoSftpPrivateKeyPassphrase, repoIdx),
.hostFingerprint = cfgOptionIdxStrNull(cfgOptRepoSftpHostFingerprint, repoIdx),
.hostKeyCheckType = cfgOptionIdxStrId(cfgOptRepoSftpHostKeyCheckType, repoIdx),
.knownHosts = strLstNewVarLst(cfgOptionIdxLst(cfgOptRepoSftpKnownHost, repoIdx)), .write = true);
((StorageSftp *)storageDriver(storageTest))->interface.pathSync = malloc(1);
TEST_ASSIGN(file, storageNewWriteP(storageTest, fileName, .noAtomic = true, .noSyncPath = false), "new write file");
((StorageWriteSftp *)ioWriteDriver(storageWriteIo(file)))->interface.syncFile = false;
TEST_RESULT_VOID(ioWriteOpen(storageWriteIo(file)), "open file");
TEST_RESULT_INT(ioWriteFd(storageWriteIo(file)), -1, "check write fd");
TEST_RESULT_VOID(ioWriteClose(storageWriteIo(file)), "close file");
free(((StorageSftp *)storageDriver(storageTest))->interface.pathSync);
memContextFree(objMemContext((StorageSftp *)storageDriver(storageTest)));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("storageWriteSftpClose() - null sftpHandle");