From 65baac7c775f45f081bf934d8e5e0c575d35b901 Mon Sep 17 00:00:00 2001 From: Michael Kazakov Date: Wed, 5 Feb 2025 20:08:11 +0000 Subject: [PATCH] Migrated Host::SetTimes() to Error --- .../source/AttrsChanging/AttrsChanging.h | 2 +- .../source/AttrsChanging/AttrsChanging.mm | 4 +-- .../source/AttrsChanging/AttrsChangingJob.cpp | 6 ++--- .../source/AttrsChanging/AttrsChangingJob.h | 4 +-- Source/VFS/include/VFS/Host.h | 12 ++++----- Source/VFS/source/Host.cpp | 14 +++++----- Source/VFS/source/Native/Host.h | 12 ++++----- Source/VFS/source/Native/Host.mm | 26 +++++++++---------- Source/VFS/source/NetSFTP/SFTPHost.cpp | 24 ++++++++--------- Source/VFS/source/NetSFTP/SFTPHost.h | 12 ++++----- Source/VFS/tests/Host_UT.cpp | 2 ++ 11 files changed, 60 insertions(+), 58 deletions(-) diff --git a/Source/Operations/source/AttrsChanging/AttrsChanging.h b/Source/Operations/source/AttrsChanging/AttrsChanging.h index 2dbbe054d..94fc9f3d0 100644 --- a/Source/Operations/source/AttrsChanging/AttrsChanging.h +++ b/Source/Operations/source/AttrsChanging/AttrsChanging.h @@ -20,7 +20,7 @@ class AttrsChanging : public Operation int OnChmodError(Error _err, const std::string &_path, VFSHost &_vfs); int OnChownError(Error _err, const std::string &_path, VFSHost &_vfs); int OnFlagsError(Error _err, const std::string &_path, VFSHost &_vfs); - int OnTimesError(int _err, const std::string &_path, VFSHost &_vfs); + int OnTimesError(Error _err, const std::string &_path, VFSHost &_vfs); std::unique_ptr m_Job; bool m_SkipAll = false; diff --git a/Source/Operations/source/AttrsChanging/AttrsChanging.mm b/Source/Operations/source/AttrsChanging/AttrsChanging.mm index 2d0a72f6e..402888bf7 100644 --- a/Source/Operations/source/AttrsChanging/AttrsChanging.mm +++ b/Source/Operations/source/AttrsChanging/AttrsChanging.mm @@ -30,7 +30,7 @@ m_Job->m_OnFlagsError = [this](Error _err, const std::string &_path, VFSHost &_vfs) { return (Callbacks::FlagsErrorResolution)OnFlagsError(_err, _path, _vfs); }; - m_Job->m_OnTimesError = [this](int _err, const std::string &_path, VFSHost &_vfs) { + m_Job->m_OnTimesError = [this](Error _err, const std::string &_path, VFSHost &_vfs) { return (Callbacks::TimesErrorResolution)OnTimesError(_err, _path, _vfs); }; auto title = NSLocalizedString(@"Altering file attributes", "Title for attributes changing operation"); @@ -148,7 +148,7 @@ return (int)Callbacks::FlagsErrorResolution::Stop; } -int AttrsChanging::OnTimesError(int _err, const std::string &_path, VFSHost &_vfs) +int AttrsChanging::OnTimesError(Error _err, const std::string &_path, VFSHost &_vfs) { if( m_SkipAll || !IsInteractive() ) return m_SkipAll ? (int)Callbacks::TimesErrorResolution::Skip : (int)Callbacks::TimesErrorResolution::Stop; diff --git a/Source/Operations/source/AttrsChanging/AttrsChangingJob.cpp b/Source/Operations/source/AttrsChanging/AttrsChangingJob.cpp index 0e45be59f..4d45521b7 100644 --- a/Source/Operations/source/AttrsChanging/AttrsChangingJob.cpp +++ b/Source/Operations/source/AttrsChanging/AttrsChangingJob.cpp @@ -284,11 +284,11 @@ bool AttrsChangingJob::ChflagSingleItem(const std::string &_path, VFSHost &_vfs, bool AttrsChangingJob::ChtimesSingleItem(const std::string &_path, VFSHost &_vfs, [[maybe_unused]] const VFSStat &_stat) { while( true ) { - const auto set_times_rc = _vfs.SetTimes( + const std::expected set_times_rc = _vfs.SetTimes( _path, m_Command.times->btime, m_Command.times->mtime, m_Command.times->ctime, m_Command.times->atime); - if( set_times_rc == VFSError::Ok ) + if( set_times_rc ) break; - switch( m_OnTimesError(set_times_rc, _path, _vfs) ) { + switch( m_OnTimesError(set_times_rc.error(), _path, _vfs) ) { case TimesErrorResolution::Stop: Stop(); return false; diff --git a/Source/Operations/source/AttrsChanging/AttrsChangingJob.h b/Source/Operations/source/AttrsChanging/AttrsChangingJob.h index 57c65db7f..c2bca3d4f 100644 --- a/Source/Operations/source/AttrsChanging/AttrsChangingJob.h +++ b/Source/Operations/source/AttrsChanging/AttrsChangingJob.h @@ -46,8 +46,8 @@ struct AttrsChangingJobCallbacks { Skip, Retry }; - std::function m_OnTimesError = - [](int, const std::string &, VFSHost &) { return TimesErrorResolution::Stop; }; + std::function m_OnTimesError = + [](Error, const std::string &, VFSHost &) { return TimesErrorResolution::Stop; }; }; class AttrsChangingJob : public Job, public AttrsChangingJobCallbacks diff --git a/Source/VFS/include/VFS/Host.h b/Source/VFS/include/VFS/Host.h index 5af792aae..9f944e367 100644 --- a/Source/VFS/include/VFS/Host.h +++ b/Source/VFS/include/VFS/Host.h @@ -296,12 +296,12 @@ class Host : public std::enable_shared_from_this /** * Adjust file node times. */ - virtual int SetTimes(std::string_view _path, - std::optional _birth_time, - std::optional _mod_time, - std::optional _chg_time, - std::optional _acc_time, - const VFSCancelChecker &_cancel_checker = {}); + virtual std::expected SetTimes(std::string_view _path, + std::optional _birth_time, + std::optional _mod_time, + std::optional _chg_time, + std::optional _acc_time, + const VFSCancelChecker &_cancel_checker = {}); /** * Change permissions similarly to chmod(). diff --git a/Source/VFS/source/Host.cpp b/Source/VFS/source/Host.cpp index aae25f788..3459acf9c 100644 --- a/Source/VFS/source/Host.cpp +++ b/Source/VFS/source/Host.cpp @@ -299,14 +299,14 @@ int Host::CreateSymlink([[maybe_unused]] std::string_view _symlink_path, return VFSError::NotSupported; } -int Host::SetTimes([[maybe_unused]] std::string_view _path, - [[maybe_unused]] std::optional _birth_time, - [[maybe_unused]] std::optional _mod_time, - [[maybe_unused]] std::optional _chg_time, - [[maybe_unused]] std::optional _acc_time, - [[maybe_unused]] const VFSCancelChecker &_cancel_checker) +std::expected Host::SetTimes([[maybe_unused]] std::string_view _path, + [[maybe_unused]] std::optional _birth_time, + [[maybe_unused]] std::optional _mod_time, + [[maybe_unused]] std::optional _chg_time, + [[maybe_unused]] std::optional _acc_time, + [[maybe_unused]] const VFSCancelChecker &_cancel_checker) { - return VFSError::NotSupported; + return std::unexpected(nc::Error{nc::Error::POSIX, ENOTSUP}); } bool Host::ShouldProduceThumbnails() const diff --git a/Source/VFS/source/Native/Host.h b/Source/VFS/source/Native/Host.h index ac132ddf8..8823eec68 100644 --- a/Source/VFS/source/Native/Host.h +++ b/Source/VFS/source/Native/Host.h @@ -89,12 +89,12 @@ class NativeHost : public Host unsigned _gid, const VFSCancelChecker &_cancel_checker = {}) override; - int SetTimes(std::string_view _path, - std::optional _birth_time, - std::optional _mod_time, - std::optional _chg_time, - std::optional _acc_time, - const VFSCancelChecker &_cancel_checker = {}) override; + std::expected SetTimes(std::string_view _path, + std::optional _birth_time, + std::optional _mod_time, + std::optional _chg_time, + std::optional _acc_time, + const VFSCancelChecker &_cancel_checker = {}) override; std::expected, Error> FetchUsers(const VFSCancelChecker &_cancel_checker = {}) override; diff --git a/Source/VFS/source/Native/Host.mm b/Source/VFS/source/Native/Host.mm index 60f68f7fc..9a500631d 100644 --- a/Source/VFS/source/Native/Host.mm +++ b/Source/VFS/source/Native/Host.mm @@ -671,33 +671,33 @@ static int CalculateDirectoriesSizesHelper(char *_path, return 0; } -int NativeHost::SetTimes(std::string_view _path, - std::optional _birth_time, - std::optional _mod_time, - std::optional _chg_time, - std::optional _acc_time, - [[maybe_unused]] const VFSCancelChecker &_cancel_checker) +std::expected NativeHost::SetTimes(const std::string_view _path, + const std::optional _birth_time, + const std::optional _mod_time, + const std::optional _chg_time, + const std::optional _acc_time, + [[maybe_unused]] const VFSCancelChecker &_cancel_checker) { if( _path.empty() ) - return VFSError::InvalidCall; + return std::unexpected(nc::Error{nc::Error::POSIX, EINVAL}); if( !_birth_time && !_mod_time && !_chg_time && !_acc_time ) - return VFSError::Ok; + return {}; StackAllocator alloc; const std::pmr::string path(_path, &alloc); auto &io = routedio::RoutedIO::Default; if( _birth_time && io.chbtime(path.c_str(), *_birth_time) != 0 ) - return VFSError::FromErrno(); + return std::unexpected(nc::Error{nc::Error::POSIX, errno}); if( _mod_time && io.chmtime(path.c_str(), *_mod_time) != 0 ) - return VFSError::FromErrno(); + return std::unexpected(nc::Error{nc::Error::POSIX, errno}); if( _chg_time && io.chctime(path.c_str(), *_chg_time) != 0 ) - return VFSError::FromErrno(); + return std::unexpected(nc::Error{nc::Error::POSIX, errno}); if( _acc_time && io.chatime(path.c_str(), *_acc_time) != 0 ) - return VFSError::FromErrno(); + return std::unexpected(nc::Error{nc::Error::POSIX, errno}); - return VFSError::Ok; + return {}; } int NativeHost::Rename(std::string_view _old_path, diff --git a/Source/VFS/source/NetSFTP/SFTPHost.cpp b/Source/VFS/source/NetSFTP/SFTPHost.cpp index 179e7b31b..7b09f9d27 100644 --- a/Source/VFS/source/NetSFTP/SFTPHost.cpp +++ b/Source/VFS/source/NetSFTP/SFTPHost.cpp @@ -914,22 +914,22 @@ std::expected SFTPHost::SetOwnership(std::string_view _path, return std::unexpected(ErrorForConnection(*conn).value_or(Error{ErrorDomain, Errors::sftp_protocol})); } -int SFTPHost::SetTimes(std::string_view _path, - std::optional _birth_time, - std::optional _mod_time, - std::optional _chg_time, - std::optional _acc_time, - [[maybe_unused]] const VFSCancelChecker &_cancel_checker) +std::expected SFTPHost::SetTimes(std::string_view _path, + std::optional _birth_time, + std::optional _mod_time, + std::optional _chg_time, + std::optional _acc_time, + [[maybe_unused]] const VFSCancelChecker &_cancel_checker) { _birth_time = std::nullopt; _chg_time = std::nullopt; if( !_birth_time && !_mod_time && !_chg_time && !_acc_time ) - return VFSError::Ok; + return {}; std::unique_ptr conn; if( const int rc = GetConnection(conn); rc < 0 ) - return rc; + return std::unexpected(VFSError::ToError(rc)); const AutoConnectionReturn acr(conn, this); @@ -938,7 +938,7 @@ int SFTPHost::SetTimes(std::string_view _path, const int rc = libssh2_sftp_stat_ex( conn->sftp, _path.data(), static_cast(_path.length()), LIBSSH2_SFTP_LSTAT, &attrs); if( rc != 0 ) - return VFSErrorForConnection(*conn); + return std::unexpected(ErrorForConnection(*conn).value_or(Error{ErrorDomain, Errors::sftp_protocol})); if( attrs.flags & LIBSSH2_SFTP_ATTR_ACMODTIME ) { if( !_mod_time ) @@ -947,7 +947,7 @@ int SFTPHost::SetTimes(std::string_view _path, _acc_time = attrs.atime; } else - return VFSError::NotSupported; + return std::unexpected(nc::Error{nc::Error::POSIX, ENOTSUP}); } LIBSSH2_SFTP_ATTRIBUTES attrs; @@ -959,9 +959,9 @@ int SFTPHost::SetTimes(std::string_view _path, const auto rc = libssh2_sftp_stat_ex( conn->sftp, _path.data(), static_cast(_path.length()), LIBSSH2_SFTP_SETSTAT, &attrs); if( rc == 0 ) - return VFSError::Ok; + return {}; else - return VFSErrorForConnection(*conn); + return std::unexpected(ErrorForConnection(*conn).value_or(Error{ErrorDomain, Errors::sftp_protocol})); } std::expected, Error> diff --git a/Source/VFS/source/NetSFTP/SFTPHost.h b/Source/VFS/source/NetSFTP/SFTPHost.h index cbf6bfca3..5ae629e11 100644 --- a/Source/VFS/source/NetSFTP/SFTPHost.h +++ b/Source/VFS/source/NetSFTP/SFTPHost.h @@ -85,12 +85,12 @@ class SFTPHost final : public Host unsigned _gid, const VFSCancelChecker &_cancel_checker = {}) override; - int SetTimes(std::string_view _path, - std::optional _birth_time, - std::optional _mod_time, - std::optional _chg_time, - std::optional _acc_time, - const VFSCancelChecker &_cancel_checker = {}) override; + std::expected SetTimes(std::string_view _path, + std::optional _birth_time, + std::optional _mod_time, + std::optional _chg_time, + std::optional _acc_time, + const VFSCancelChecker &_cancel_checker = {}) override; std::expected, Error> FetchUsers(const VFSCancelChecker &_cancel_checker = {}) override; diff --git a/Source/VFS/tests/Host_UT.cpp b/Source/VFS/tests/Host_UT.cpp index 3cb5b15d1..f0a6d86b4 100644 --- a/Source/VFS/tests/Host_UT.cpp +++ b/Source/VFS/tests/Host_UT.cpp @@ -72,6 +72,8 @@ TEST_CASE(PREFIX "Unsupported methods") REQUIRE(host->SetFlags("/some/path", 0, 0).error() == Error(Error::POSIX, ENOTSUP)); REQUIRE(host->SetPermissions("/some/path", 42).error() == Error(Error::POSIX, ENOTSUP)); REQUIRE(host->SetOwnership("/some/path", 42, 42).error() == Error(Error::POSIX, ENOTSUP)); + REQUIRE(host->SetTimes("/some/path", std::nullopt, std::nullopt, std::nullopt, std::nullopt).error() == + Error(Error::POSIX, ENOTSUP)); REQUIRE(host->FetchUsers().error() == Error(Error::POSIX, ENOTSUP)); REQUIRE(host->FetchGroups().error() == Error(Error::POSIX, ENOTSUP)); // ...