Migrated Host::SetTimes() to Error

mikekazakov · Feb 5, 2025 · 65baac7 · 65baac7
1 parent c69256a
commit 65baac7
Show file tree

Hide file tree

Showing 11 changed files with 60 additions and 58 deletions.
diff --git 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<AttrsChangingJob> m_Job;
     bool m_SkipAll = false;

diff --git 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
@@ -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<void, Error> 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
@@ -46,8 +46,8 @@ struct AttrsChangingJobCallbacks {
         Skip,
         Retry
     };
-    std::function<TimesErrorResolution(int _err, const std::string &_path, VFSHost &_vfs)> m_OnTimesError =
-        [](int, const std::string &, VFSHost &) { return TimesErrorResolution::Stop; };
+    std::function<TimesErrorResolution(Error _err, const std::string &_path, VFSHost &_vfs)> 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
@@ -296,12 +296,12 @@ class Host : public std::enable_shared_from_this<Host>
     /**
      * Adjust file node times.
      */
-    virtual int SetTimes(std::string_view _path,
-                         std::optional<time_t> _birth_time,
-                         std::optional<time_t> _mod_time,
-                         std::optional<time_t> _chg_time,
-                         std::optional<time_t> _acc_time,
-                         const VFSCancelChecker &_cancel_checker = {});
+    virtual std::expected<void, Error> SetTimes(std::string_view _path,
+                                                std::optional<time_t> _birth_time,
+                                                std::optional<time_t> _mod_time,
+                                                std::optional<time_t> _chg_time,
+                                                std::optional<time_t> _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
@@ -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<time_t> _birth_time,
-                   [[maybe_unused]] std::optional<time_t> _mod_time,
-                   [[maybe_unused]] std::optional<time_t> _chg_time,
-                   [[maybe_unused]] std::optional<time_t> _acc_time,
-                   [[maybe_unused]] const VFSCancelChecker &_cancel_checker)
+std::expected<void, Error> Host::SetTimes([[maybe_unused]] std::string_view _path,
+                                          [[maybe_unused]] std::optional<time_t> _birth_time,
+                                          [[maybe_unused]] std::optional<time_t> _mod_time,
+                                          [[maybe_unused]] std::optional<time_t> _chg_time,
+                                          [[maybe_unused]] std::optional<time_t> _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
@@ -89,12 +89,12 @@ class NativeHost : public Host
                                             unsigned _gid,
                                             const VFSCancelChecker &_cancel_checker = {}) override;
 
-    int SetTimes(std::string_view _path,
-                 std::optional<time_t> _birth_time,
-                 std::optional<time_t> _mod_time,
-                 std::optional<time_t> _chg_time,
-                 std::optional<time_t> _acc_time,
-                 const VFSCancelChecker &_cancel_checker = {}) override;
+    std::expected<void, Error> SetTimes(std::string_view _path,
+                                        std::optional<time_t> _birth_time,
+                                        std::optional<time_t> _mod_time,
+                                        std::optional<time_t> _chg_time,
+                                        std::optional<time_t> _acc_time,
+                                        const VFSCancelChecker &_cancel_checker = {}) override;
 
     std::expected<std::vector<VFSUser>, Error> FetchUsers(const VFSCancelChecker &_cancel_checker = {}) override;
 

diff --git 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<time_t> _birth_time,
-                         std::optional<time_t> _mod_time,
-                         std::optional<time_t> _chg_time,
-                         std::optional<time_t> _acc_time,
-                         [[maybe_unused]] const VFSCancelChecker &_cancel_checker)
+std::expected<void, Error> NativeHost::SetTimes(const std::string_view _path,
+                                                const std::optional<time_t> _birth_time,
+                                                const std::optional<time_t> _mod_time,
+                                                const std::optional<time_t> _chg_time,
+                                                const std::optional<time_t> _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
@@ -914,22 +914,22 @@ std::expected<void, Error> 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<time_t> _birth_time,
-                       std::optional<time_t> _mod_time,
-                       std::optional<time_t> _chg_time,
-                       std::optional<time_t> _acc_time,
-                       [[maybe_unused]] const VFSCancelChecker &_cancel_checker)
+std::expected<void, Error> SFTPHost::SetTimes(std::string_view _path,
+                                              std::optional<time_t> _birth_time,
+                                              std::optional<time_t> _mod_time,
+                                              std::optional<time_t> _chg_time,
+                                              std::optional<time_t> _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<Connection> 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<unsigned>(_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<unsigned>(_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<std::vector<VFSUser>, Error>

diff --git 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<time_t> _birth_time,
-                 std::optional<time_t> _mod_time,
-                 std::optional<time_t> _chg_time,
-                 std::optional<time_t> _acc_time,
-                 const VFSCancelChecker &_cancel_checker = {}) override;
+    std::expected<void, Error> SetTimes(std::string_view _path,
+                                        std::optional<time_t> _birth_time,
+                                        std::optional<time_t> _mod_time,
+                                        std::optional<time_t> _chg_time,
+                                        std::optional<time_t> _acc_time,
+                                        const VFSCancelChecker &_cancel_checker = {}) override;
 
     std::expected<std::vector<VFSUser>, Error> FetchUsers(const VFSCancelChecker &_cancel_checker = {}) override;
 

diff --git 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));
     // ...