Hi Minh ack with minor comments:
- it seems like we have the wrong data structure here, maybe fix in an enhancement.
- try to simplify the else statement, eg a single place that calls dequeue/queue?
On 10/12/18 4:44 pm, Minh Chau wrote:
--- src/amf/amfd/imm.cc | 54 ++++++++++++++++++---------------------------------- src/amf/amfd/imm.h | 2 -- src/amf/amfd/role.cc | 2 +- 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/src/amf/amfd/imm.cc b/src/amf/amfd/imm.cc index 82d2b13..d917b0d 100644 --- a/src/amf/amfd/imm.cc +++ b/src/amf/amfd/imm.cc @@ -456,16 +456,19 @@ AvdJobDequeueResultT Fifo::executeAll(AVD_CL_CB *cb, AvdJobTypeT job_type) { if (ret != JOB_EXECUTED) break; } else { - // push back - ajob = Fifo::dequeue(); - Fifo::queue(ajob); - // check if we have gone through all jobs of queue if (firstjob == nullptr) { firstjob = ajob; + // push back + ajob = Fifo::dequeue(); + Fifo::queue(ajob); } else { - if (firstjob == ajob) - break; + if (firstjob == ajob) break; + else { + // push back + ajob = Fifo::dequeue(); + Fifo::queue(ajob); + } } } } @@ -476,54 +479,33 @@ AvdJobDequeueResultT Fifo::executeAll(AVD_CL_CB *cb, AvdJobTypeT job_type) { }void Fifo::remove(const AVD_CL_CB *cb, AvdJobTypeT job_type) {- Job *ajob, *firstjob;TRACE_ENTER();firstjob = nullptr; - while ((ajob = peek()) != nullptr) { if (ajob->getJobType() == job_type) { delete Fifo::dequeue(); } else { - // push back - ajob = Fifo::dequeue(); - Fifo::queue(ajob); - // check if we have gone through all jobs of queue if (firstjob == nullptr) { firstjob = ajob; + // push back + ajob = Fifo::dequeue(); + Fifo::queue(ajob); } else { - if (firstjob == ajob) - break; + if (firstjob == ajob) break; + else { + // push back + ajob = Fifo::dequeue(); + Fifo::queue(ajob); + } } } } - TRACE_LEAVE(); }-AvdJobDequeueResultT Fifo::executeAdminResp(AVD_CL_CB *cb) {- Job *ajob; - AvdJobDequeueResultT ret = JOB_EXECUTED; - - TRACE_ENTER(); - - while ((ajob = peek()) != nullptr) { - if (dynamic_cast<ImmAdminResponse *>(ajob) != nullptr) { - ret = ajob->exec(cb); - } else { - ajob = dequeue(); - delete ajob; - ret = JOB_EXECUTED; - } - } - - TRACE_LEAVE2("%d", ret); - - return ret; -} - // void Fifo::empty() { Job *ajob; diff --git a/src/amf/amfd/imm.h b/src/amf/amfd/imm.h index 3cfc207..670c691 100644 --- a/src/amf/amfd/imm.h +++ b/src/amf/amfd/imm.h @@ -169,8 +169,6 @@ class Fifo { AvdJobTypeT job_type = JOB_TYPE_ANY); static void remove(const AVD_CL_CB *cb, AvdJobTypeT job_type = JOB_TYPE_ANY); - static AvdJobDequeueResultT executeAdminResp(AVD_CL_CB *cb); - static void empty();static uint32_t size();diff --git a/src/amf/amfd/role.cc b/src/amf/amfd/role.cc index 42f77f8..15b0458 100644 --- a/src/amf/amfd/role.cc +++ b/src/amf/amfd/role.cc @@ -802,7 +802,7 @@ try_again: /* Execute admin op jobs before calling saImmOiImplementerClear to avoid * SA_AIS_ERR_TIMEOUT */ - Fifo::executeAdminResp(cb); + Fifo::executeAll(cb, JOB_TYPE_IMM);/* Take mutex here to sync with imm reinit thread.*/osaf_mutex_lock_ordie(&imm_reinit_mutex);
_______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
