pespin has submitted this change and it was merged. (
https://gerrit.osmocom.org/c/osmo-sgsn/+/15186 )
Change subject: gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type
......................................................................
gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type
For new readers it's very confusing why PMM states and MM states are in
the same enum, but handled with different functions, and sometimes
called one right after the other with different enums. Calling them when
on a different ran_type makes the function early return, so let's better
conditionally call the function to make it clear in the flow when the
function is expected to do something.
Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73
---
M src/gprs/gprs_gmm.c
1 file changed, 27 insertions(+), 14 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
osmith: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/gprs/gprs_gmm.c b/src/gprs/gprs_gmm.c
index 95a1842..6c87032 100644
--- a/src/gprs/gprs_gmm.c
+++ b/src/gprs/gprs_gmm.c
@@ -138,8 +138,7 @@
static void mmctx_set_pmm_state(struct sgsn_mm_ctx *ctx, enum gprs_pmm_state
state)
{
- if (ctx->ran_type != MM_CTX_T_UTRAN_Iu)
- return;
+ OSMO_ASSERT(ctx->ran_type == MM_CTX_T_UTRAN_Iu);
if (ctx->pmm_state == state)
return;
@@ -164,8 +163,7 @@
static void mmctx_set_mm_state(struct sgsn_mm_ctx *ctx, enum gprs_pmm_state
state)
{
- if (ctx->ran_type != MM_CTX_T_GERAN_Gb)
- return;
+ OSMO_ASSERT(ctx->ran_type == MM_CTX_T_GERAN_Gb);
if (ctx->pmm_state == state)
return;
@@ -326,8 +324,15 @@
/* Mark MM state as deregistered */
ctx->gmm_state = GMM_DEREGISTERED;
- mmctx_set_pmm_state(ctx, PMM_DETACHED);
- mmctx_set_mm_state(ctx, MM_IDLE);
+
+ switch(ctx->ran_type) {
+ case MM_CTX_T_UTRAN_Iu:
+ mmctx_set_pmm_state(ctx, PMM_DETACHED);
+ break;
+ case MM_CTX_T_GERAN_Gb:
+ mmctx_set_mm_state(ctx, MM_IDLE);
+ break;
+ }
sgsn_mm_ctx_cleanup_free(ctx);
}
@@ -2078,16 +2083,20 @@
mmctx->t3350_mode = GMM_T3350_MODE_NONE;
mmctx->p_tmsi_old = 0;
mmctx->pending_req = 0;
- if (mmctx->ran_type == MM_CTX_T_GERAN_Gb) {
+ mmctx->gmm_state = GMM_REGISTERED_NORMAL;
+ switch(mmctx->ran_type) {
+ case MM_CTX_T_UTRAN_Iu:
+ mmctx_set_pmm_state(mmctx, PMM_CONNECTED);
+ break;
+ case MM_CTX_T_GERAN_Gb:
/* Unassign the old TLLI */
mmctx->gb.tlli = mmctx->gb.tlli_new;
gprs_llme_copy_key(mmctx, mmctx->gb.llme);
gprs_llgmm_assign(mmctx->gb.llme, TLLI_UNASSIGNED,
mmctx->gb.tlli_new);
+ mmctx_set_mm_state(mmctx, MM_READY);
+ break;
}
- mmctx->gmm_state = GMM_REGISTERED_NORMAL;
- mmctx_set_pmm_state(mmctx, PMM_CONNECTED);
- mmctx_set_mm_state(mmctx, MM_READY);
rc = 0;
osmo_fsm_inst_dispatch(mmctx->gmm_att_req.fsm,
E_ATTACH_COMPLETE_RECV, 0);
@@ -2104,15 +2113,19 @@
mmctx->t3350_mode = GMM_T3350_MODE_NONE;
mmctx->p_tmsi_old = 0;
mmctx->pending_req = 0;
- if (mmctx->ran_type == MM_CTX_T_GERAN_Gb) {
+ mmctx->gmm_state = GMM_REGISTERED_NORMAL;
+ switch(mmctx->ran_type) {
+ case MM_CTX_T_UTRAN_Iu:
+ mmctx_set_pmm_state(mmctx, PMM_CONNECTED);
+ break;
+ case MM_CTX_T_GERAN_Gb:
/* Unassign the old TLLI */
mmctx->gb.tlli = mmctx->gb.tlli_new;
gprs_llgmm_assign(mmctx->gb.llme, TLLI_UNASSIGNED,
mmctx->gb.tlli_new);
+ mmctx_set_mm_state(mmctx, MM_READY);
+ break;
}
- mmctx->gmm_state = GMM_REGISTERED_NORMAL;
- mmctx_set_pmm_state(mmctx, PMM_CONNECTED);
- mmctx_set_mm_state(mmctx, MM_READY);
rc = 0;
memset(&sig_data, 0, sizeof(sig_data));
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15186
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73
Gerrit-Change-Number: 15186
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-CC: neels <[email protected]>
Gerrit-MessageType: merged
