pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/30219 )

Change subject: Refactor tbf_is_tfi_assigned() to avoid accessing tbf->state_fsm ......................................................................

Refactor tbf_is_tfi_assigned() to avoid accessing tbf->state_fsm

The state_fsm field will be moved to subclass (DL_TBF/UL_TBF) in a follow up commit when splitting the tbf_fsm.c implementation per subclass. Rearrange a bit the code to access the using the subclass pointer in the only sublcass where it needs to be used (DL_TBF).

Change-Id: I360485c73be8636565f89ba29796d84ac94fd94e --- M src/gprs_rlcmac_sched.cpp M src/tbf.cpp M src/tbf.h 3 files changed, 15 insertions(+), 14 deletions(-)

Approvals: Jenkins Builder: Verified msuraev: Looks good to me, but someone else must approve pespin: Looks good to me, approved laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve

diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp index 20b4f8e..1857c3c 100644 --- a/src/gprs_rlcmac_sched.cpp +++ b/src/gprs_rlcmac_sched.cpp @@ -57,7 +57,7 @@ if (tbf_ul_ass_rts(ul_tbf)) tbf_cand->ul_ass = ul_tbf; /* NACC ready to send. TFI assigned is needed to send messages */ - if (ul_tbf->is_tfi_assigned() && ms_nacc_rts(ul_tbf->ms())) + if (tbf_is_tfi_assigned(ul_tbf) && ms_nacc_rts(ul_tbf->ms())) tbf_cand->nacc = ul_tbf; /* FIXME: Is this supposed to be fair? The last TBF for each wins? Maybe use llist_add_tail and skip once we have all states? */ @@ -73,7 +73,7 @@ if (tbf_ul_ass_rts(dl_tbf)) tbf_cand->ul_ass = dl_tbf; /* NACC ready to send. TFI assigned is needed to send messages */ - if (dl_tbf->is_tfi_assigned() && ms_nacc_rts(dl_tbf->ms())) + if (tbf_is_tfi_assigned(dl_tbf) && ms_nacc_rts(dl_tbf->ms())) tbf_cand->nacc = dl_tbf; } } diff --git a/src/tbf.cpp b/src/tbf.cpp index 87e85f0..4aa4089 100644 --- a/src/tbf.cpp +++ b/src/tbf.cpp @@ -851,7 +851,19 @@

bool tbf_is_tfi_assigned(const struct gprs_rlcmac_tbf *tbf) { - return tbf->is_tfi_assigned(); + const struct gprs_rlcmac_dl_tbf *dl_tbf; + + /* The TBF is established: */ + if (tbf->state_fi->state > TBF_ST_ASSIGN) + return true; + + /* The DL TBF has been assigned by a IMM.ASS: */ + dl_tbf = tbf_as_dl_tbf_const(tbf); + if (tbf->state_fi->state == TBF_ST_ASSIGN && dl_tbf && + (dl_tbf->state_fsm.state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) + return true; + + return false; }

uint8_t tbf_tfi(const struct gprs_rlcmac_tbf *tbf) diff --git a/src/tbf.h b/src/tbf.h index fedf312..b1ff221 100644 --- a/src/tbf.h +++ b/src/tbf.h @@ -200,7 +200,6 @@ bool is_tlli_valid() const;

uint8_t tfi() const; - bool is_tfi_assigned() const;

const char *imsi() const; uint8_t ta() const; @@ -315,16 +314,6 @@ return tlli() != GSM_RESERVED_TMSI; }

-inline bool gprs_rlcmac_tbf::is_tfi_assigned() const -{ - /* The TBF is established or has been assigned by a IMM.ASS for - * download */ - return state_fi->state > TBF_ST_ASSIGN || - (direction == GPRS_RLCMAC_DL_TBF && - state_fi->state == TBF_ST_ASSIGN && - (state_fsm.state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))); -} - inline uint8_t gprs_rlcmac_tbf::tfi() const { return m_tfi;