pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/40278?usp=email )
Change subject: kpi: Increment counters using enum field explicitly ......................................................................
kpi: Increment counters using enum field explicitly
I was first tricked by the increment of ctr_num. This is wrong for several reasons: * Assumes one counter will always be immediatelly after the other, and there's no static assert about that. * Makes it difficult to grep for users of the counter in the code.
Change-Id: I126973b93e86784adaccbcba49c6322c261f4746 --- M src/osmo-hnbgw/kpi_ranap.c 1 file changed, 8 insertions(+), 12 deletions(-)
Approvals: Jenkins Builder: Verified osmith: Looks good to me, approved
diff --git a/src/osmo-hnbgw/kpi_ranap.c b/src/osmo-hnbgw/kpi_ranap.c index c1c77bf..640f838 100644 --- a/src/osmo-hnbgw/kpi_ranap.c +++ b/src/osmo-hnbgw/kpi_ranap.c @@ -52,18 +52,16 @@
/* When Iu is released, all RABs are released implicitly */ for (unsigned int i = 0; i < ARRAY_SIZE(map->rab_state); i++) { - unsigned int ctr_num; switch (map->rab_state[i]) { case RAB_STATE_ACTIVE: if (cause->present == RANAP_Cause_PR_nAS || cause->choice.nAS == RANAP_CauseNAS_normal_release) { - ctr_num = HNB_CTR_RANAP_PS_RAB_REL_IMPLICIT; + HNBP_CTR_INC(hnbp, map->is_ps ? HNB_CTR_RANAP_PS_RAB_REL_IMPLICIT : + HNB_CTR_RANAP_CS_RAB_REL_IMPLICIT); } else { - ctr_num = HNB_CTR_RANAP_PS_RAB_REL_IMPLICIT_ABNORMAL; + HNBP_CTR_INC(hnbp, map->is_ps ? HNB_CTR_RANAP_PS_RAB_REL_IMPLICIT_ABNORMAL : + HNB_CTR_RANAP_CS_RAB_REL_IMPLICIT_ABNORMAL); } - if (!map->is_ps) - ctr_num++; - HNBP_CTR_INC(hnbp, ctr_num); break; } } @@ -132,7 +130,6 @@ RANAP_RAB_ReleaseItemIEs_t _rab_rel_item_ies = {}; RANAP_RAB_ReleaseItemIEs_t *rab_rel_item_ies = &_rab_rel_item_ies; RANAP_RAB_ReleaseItem_t *rab_rel_item; - unsigned int ctr_num; uint8_t rab_id;
if (!release_list_ie) @@ -153,13 +150,12 @@ case RAB_STATE_ACTIVE: if (rab_rel_item->cause.present == RANAP_Cause_PR_nAS && rab_rel_item->cause.choice.nAS == RANAP_CauseNAS_normal_release) { - ctr_num = HNB_CTR_RANAP_PS_RAB_REL_REQ; + HNBP_CTR_INC(hnbp, map->is_ps ? HNB_CTR_RANAP_PS_RAB_REL_REQ : + HNB_CTR_RANAP_CS_RAB_REL_REQ); } else { - ctr_num = HNB_CTR_RANAP_PS_RAB_REL_REQ_ABNORMAL; + HNBP_CTR_INC(hnbp, map->is_ps ? HNB_CTR_RANAP_PS_RAB_REL_REQ_ABNORMAL : + HNB_CTR_RANAP_CS_RAB_REL_REQ_ABNORMAL); } - if (!map->is_ps) - ctr_num++; - HNBP_CTR_INC(hnbp, ctr_num); break; default: LOG_MAP(map, DRANAP, LOGL_NOTICE,