diff options
| author | Neels Hofmeyr <neels@hofmeyr.de> | 2018-08-27 22:06:24 +0200 |
|---|---|---|
| committer | Neels Hofmeyr <neels@hofmeyr.de> | 2018-08-29 02:02:10 +0200 |
| commit | 193c1e3ab09af9fd9ed970274af8f0872c9696f6 (patch) | |
| tree | 93cd68efdd92d5bdc778e55efbbcfe9e461a60c8 /src | |
| parent | e23e1aeec2206e56cb4bd3f0bffb39eef44c0e41 (diff) | |
lchan_fsm: safer 'concluded' flag
The flag lchan->activate.concluded prevents entering the
lchan_on_fully_established()/lchan_on_activation_failure() more than once. So
far it was checked by callers, instead place in the functions themselves.
There is a potential functional change here, since during lchan_fail(), the
concluded flag was set only after entering lchan_on_activation_failure(). Now
it is already set at the start and prevents multiple re-entry beyond doubt.
This patch is not a result of an actual observed faulty behavior, just from
reading the code and seeing the slight loophole.
Change-Id: I0c906438b05442d66255203eb816453b7193a6d8
Diffstat (limited to 'src')
| -rw-r--r-- | src/osmo-bsc/lchan_fsm.c | 16 |
1 files changed, 9 insertions, 7 deletions
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c index e856fd0fa..13ae7ad6d 100644 --- a/src/osmo-bsc/lchan_fsm.c +++ b/src/osmo-bsc/lchan_fsm.c @@ -88,6 +88,10 @@ static void _lchan_on_activation_failure(struct gsm_lchan *lchan, enum lchan_act struct gsm_subscriber_connection *for_conn, const char *file, int line) { + if (lchan->activate.concluded) + return; + lchan->activate.concluded = true; + switch (activ_for) { case FOR_MS_CHANNEL_REQUEST: @@ -135,6 +139,10 @@ static void _lchan_on_activation_failure(struct gsm_lchan *lchan, enum lchan_act static void lchan_on_fully_established(struct gsm_lchan *lchan) { + if (lchan->activate.concluded) + return; + lchan->activate.concluded = true; + switch (lchan->activate.activ_for) { case FOR_MS_CHANNEL_REQUEST: /* No signalling to do here, MS is free to use the channel, and should go on to connect @@ -222,9 +230,7 @@ struct state_timeout lchan_fsm_timeouts[32] = { lchan_set_last_error(_lchan, "lchan %s in state %s: " fmt, \ _lchan->activate.concluded ? "failure" : "allocation failed", \ osmo_fsm_state_name(fsm, state_was), ## args); \ - if (!_lchan->activate.concluded) \ - lchan_on_activation_failure(_lchan, _lchan->activate.activ_for, _lchan->conn); \ - _lchan->activate.concluded = true; \ + lchan_on_activation_failure(_lchan, _lchan->activate.activ_for, _lchan->conn); \ if (fi->state != state_chg) \ lchan_fsm_state_chg(state_chg); \ else \ @@ -751,10 +757,6 @@ static void lchan_fsm_established_onenter(struct osmo_fsm_inst *fi, uint32_t pre return; } - /* This flag ensures that when an lchan activation has succeeded, and we have already sent ACKs - * like Immediate Assignment or BSSMAP Assignment Complete, and if then, way later, some other - * error occurs, e.g. during release, that we don't send a NACK out of context. */ - lchan->activate.concluded = true; lchan_on_fully_established(lchan); } |