From: Matan Breizman <mbreizma@redhat.com>
Date: Thu, 10 Apr 2025 14:23:50 +0000 (+0000)
Subject: crimson/common/errorator: Always check exception type
X-Git-Tag: v20.3.0~28^2~10
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bb717e2b69ab2948aa9722bf0c05143981c5dfe9;p=ceph.git

crimson/common/errorator: Always check exception type

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>
---

diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h
index 75bce8d663d6..67a0c18b741d 100644
--- a/src/crimson/common/errorator.h
+++ b/src/crimson/common/errorator.h
@@ -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)));
       }
     }
   }