From: Kefu Chai <kchai@redhat.com>
Date: Wed, 21 Jul 2021 04:36:12 +0000 (+0800)
Subject: mon: let CrushWrapper::get_validated_type_id() return an optional<>
X-Git-Tag: v17.1.0~1256^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b35f4ed81865eacd34e81dd4d1368ea092f51523;p=ceph.git

mon: let CrushWrapper::get_validated_type_id() return an optional<>

easier to reason about. and it helps to silence the false alarm of

../src/mon/OSDMonitor.cc: In member function 'void OSDMonitor::try_enable_stretch_mode(std::stringstream&, bool*, int*, bool, const string&, uint32_t, const std::set<pg_pool_t*>&, const string&)':
../src/mon/OSDMonitor.cc:14379:44: warning: 'dividing_id' may be used uninitialized in this function [-Wmaybe-uninitialized]
14379 |       pool->peering_crush_bucket_barrier = dividing_id;
      |                                            ^~~~~~~~~~~

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 413e0027cb70..14295f4c6702 100644
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -378,13 +378,13 @@ public:
       return type_rmap[name];
     return -1;
   }
-  int get_validated_type_id(const std::string& name, int *id) const {
+  std::optional<int> get_validated_type_id(const std::string& name) const {
     int retval = get_type_id(name);
     if (retval == -1 && !type_rmap.count(name)) {
-      return -1;
+      return {};
+    } else {
+      return retval;
     }
-    *id = retval;
-    return 0;
   }
   const char *get_type_name(int t) const {
     auto p = type_map.find(t);
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 36647053c977..6f4a8fc0e833 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -6809,10 +6809,12 @@ bool Monitor::session_stretch_allowed(MonSession *s, MonOpRequestRef& op)
   if (s->con->peer_is_osd()) {
     dout(20) << __func__ << "checking OSD session" << s << dendl;
     // okay, check the crush location
-    int barrier_id;
-    int retval = osdmon()->osdmap.crush->get_validated_type_id(stretch_bucket_divider,
-							       &barrier_id);
-    ceph_assert(retval >= 0);
+    int barrier_id = [&] {
+      auto type_id = osdmon()->osdmap.crush->get_validated_type_id(
+	stretch_bucket_divider);
+      ceph_assert(type_id.has_value());
+      return *type_id;
+    }();
     int osd_bucket_id = osdmon()->osdmap.crush->get_parent_of_type(s->con->peer_id,
 								   barrier_id);
     const auto &mi = monmap->mon_info.find(name);
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 92aca2887d68..1960d37bd833 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -14325,13 +14325,15 @@ void OSDMonitor::try_enable_stretch_mode(stringstream& ss, bool *okay,
   dout(20) << __func__ << dendl;
   *okay = false;
   CrushWrapper crush = _get_pending_crush();
-  int dividing_id;
-  int retval = crush.get_validated_type_id(dividing_bucket, &dividing_id);
-  if (retval == -1) {
+  int dividing_id = -1;
+  if (auto type_id = crush.get_validated_type_id(dividing_bucket);
+      !type_id.has_value()) {
     ss << dividing_bucket << " is not a valid crush bucket type";
     *errcode = -ENOENT;
-    ceph_assert(!commit || retval != -1);
+    ceph_assert(!commit);
     return;
+  } else {
+    dividing_id = *type_id;
   }
   vector<int> subtrees;
   crush.get_subtree_of_type(dividing_id, &subtrees);