]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
crimson/common/errorator: Always check exception type
authorMatan Breizman <mbreizma@redhat.com>
Thu, 10 Apr 2025 14:23:50 +0000 (14:23 +0000)
committerMatan Breizman <mbreizma@redhat.com>
Tue, 22 Apr 2025 15:11:58 +0000 (15:11 +0000)
We shouldn't bypass this check in the is_same_v<return_t, no_touch_error_marker>
case.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
src/crimson/common/errorator.h

index 75bce8d663d6b9266a3d5e1d8cd30700c2bafbc7..67a0c18b741deca6e2c3bfe5f479d9f9d3062aea 100644 (file)
@@ -381,6 +381,15 @@ public:
     static_assert(!std::is_same_v<return_t, void>,
                   "error handlers mustn't return void");
 
+    // The code below checks for exact match only while
+    // `catch` would allow to match against a base class as well.
+    // However, this shouldn't be a big issue for `errorator` as
+    // ErrorVisitorT are already checked for exhaustiveness at compile-time.
+    // TODO: why/when is this possible?
+    if (type_info != ErrorT::error_t::get_exception_ptr_type_info()) {
+        return;
+    }
+
     auto ep = take_exception_from_future();
 
     // Any assert_* handler we have:
@@ -404,21 +413,16 @@ public:
       // pointee's type with `__cxa_exception_type()` instead of costly
       // re-throwing (via `std::rethrow_exception()`) and matching with
       // `catch`. The limitation here is lack of support for hierarchies
-      // of exceptions. The code below checks for exact match only while
-      // `catch` would allow to match against a base class as well.
-      // However, this shouldn't be a big issue for `errorator` as
-      // ErrorVisitorT are already checked for exhaustiveness at compile-time.
-      if (type_info == ErrorT::error_t::get_exception_ptr_type_info()) {
-
-        // TODO: add missing explanation
-        if constexpr (std::is_assignable_v<decltype(result), return_t>) {
-          result = std::invoke(std::forward<ErrorVisitorT>(errfunc),
-                               ErrorT::error_t::from_exception_ptr(std::move(ep)));
-        } else {
-          result = FuturatorT::invoke(
-            std::forward<ErrorVisitorT>(errfunc),
-            ErrorT::error_t::from_exception_ptr(std::move(ep)));
-        }
+      // of exceptions.
+
+       // TODO: add missing explanation
+      if constexpr (std::is_assignable_v<decltype(result), return_t>) {
+        result = std::invoke(std::forward<ErrorVisitorT>(errfunc),
+                             ErrorT::error_t::from_exception_ptr(std::move(ep)));
+      } else {
+        result = FuturatorT::invoke(
+          std::forward<ErrorVisitorT>(errfunc),
+          ErrorT::error_t::from_exception_ptr(std::move(ep)));
       }
     }
   }