From 2d20f3a8a4cd36da71df18c0eb58057273d6af59 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sun, 21 Oct 2012 21:07:12 -0700 Subject: [PATCH] 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 --- src/osdc/Objecter.cc | 40 +++++++++++++++++++++++++--------------- src/osdc/Objecter.h | 2 ++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index 7bbf93be09138..72690a5dd54b4 100644 --- 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 3244c7f49ca55..54fbaf3e46bd2 100644 --- 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); -- 2.39.5