objecter: move map checks to helper

This makes coverity happier because we check_op_pool_dne() may free
the Op (or Lingerop) structure(s), but the callers in the submit_*
paths dereference after calling.  This is actually safe because they
never free new ops, but is confusing.  Explicitly push this into a
separate helper.

CID 739607 (#1-2 of 2): Read from pointer after free (USE_AFTER_FREE)
At (9): Dereferencing freed pointer "o".

CID 739606 (#1 of 1): Read from pointer after free (USE_AFTER_FREE)
At (28): Dereferencing freed pointer "op".

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 7bbf93be0913804be41faa83f947ebeac4de6a44..72690a5dd54b45f03f9c2918f47c61dbf7906bca 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -264,7 +264,7 @@ void Objecter::send_linger(LingerOp *info)
   if (info->session) {
     int r = recalc_op_target(o);
     if (r == RECALC_OP_TARGET_POOL_DNE) {
-      check_linger_pool_dne(info);
+      _send_linger_map_check(info);
     }
   }
 
@@ -649,12 +649,17 @@ void Objecter::check_op_pool_dne(Op *op)
       delete op;
     }
   } else {
-    // ask the monitor
-    if (check_latest_map_ops.count(op->tid) == 0) {
-      check_latest_map_ops[op->tid] = op;
-      C_Op_Map_Latest *c = new C_Op_Map_Latest(this, op->tid);
-      monc->get_version("osdmap", &c->latest, NULL, c);
-    }
+    _send_op_map_check(op);
+  }
+}
+
+void Objecter::_send_op_map_check(Op *op)
+{
+  // ask the monitor
+  if (check_latest_map_ops.count(op->tid) == 0) {
+    check_latest_map_ops[op->tid] = op;
+    C_Op_Map_Latest *c = new C_Op_Map_Latest(this, op->tid);
+    monc->get_version("osdmap", &c->latest, NULL, c);
   }
 }
 
@@ -700,13 +705,18 @@ void Objecter::check_linger_pool_dne(LingerOp *op)
       unregister_linger(op->linger_id);
     }
   } else {
-    // ask the monitor
-    if (check_latest_map_lingers.count(op->linger_id) == 0) {
-      op->get();
-      check_latest_map_lingers[op->linger_id] = op;
-      C_Linger_Map_Latest *c = new C_Linger_Map_Latest(this, op->linger_id);
-      monc->get_version("osdmap", &c->latest, NULL, c);
-    }
+    _send_linger_map_check(op);
+  }
+}
+
+void Objecter::_send_linger_map_check(LingerOp *op)
+{
+  // ask the monitor
+  if (check_latest_map_lingers.count(op->linger_id) == 0) {
+    op->get();
+    check_latest_map_lingers[op->linger_id] = op;
+    C_Linger_Map_Latest *c = new C_Linger_Map_Latest(this, op->linger_id);
+    monc->get_version("osdmap", &c->latest, NULL, c);
   }
 }
 
@@ -1062,7 +1072,7 @@ tid_t Objecter::_op_submit(Op *op)
   }
 
   if (check_for_latest_map) {
-    check_op_pool_dne(op);
+    _send_op_map_check(op);
   }
 
   ldout(cct, 5) << num_unacked << " unacked, " << num_uncommitted << " uncommitted" << dendl;

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 3244c7f49ca5512514506ebe6565d44392773136..54fbaf3e46bd22547ff63ccc120a869281b9c575 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -902,8 +902,10 @@ public:
   void _linger_commit(LingerOp *info, int r);
 
   void check_op_pool_dne(Op *op);
+  void _send_op_map_check(Op *op);
   void op_cancel_map_check(Op *op);
   void check_linger_pool_dne(LingerOp *op);
+  void _send_linger_map_check(LingerOp *op);
   void linger_cancel_map_check(LingerOp *op);
 
   void kick_requests(OSDSession *session);