This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgNeels Hofmeyr has submitted this change and it was merged. ( https://gerrit.osmocom.org/10638 ) Change subject: lchan_fsm: safer 'concluded' flag ...................................................................... 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 --- M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/lchan_fsm.c 2 files changed, 13 insertions(+), 8 deletions(-) Approvals: Jenkins Builder: Verified Harald Welte: Looks good to me, approved diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index b827d0a..d6e8954 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -505,7 +505,10 @@ struct { enum lchan_activate_mode activ_for; bool activ_ack; /*< true as soon as RSL Chan Activ Ack is received */ - bool concluded; /*< true as soon as LCHAN_ST_ESTABLISHED is reached */ + /*! 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 other errors + * occur later, e.g. during release, that we don't send a NACK out of context. */ + bool concluded; bool requires_voice_stream; bool wait_before_switching_rtp; /*< true = requires LCHAN_EV_READY_TO_SWITCH_RTP */ uint16_t msc_assigned_cic; diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c index e856fd0..13ae7ad 100644 --- a/src/osmo-bsc/lchan_fsm.c +++ b/src/osmo-bsc/lchan_fsm.c @@ -88,6 +88,10 @@ 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_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 @@ 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 @@ 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); } -- To view, visit https://gerrit.osmocom.org/10638 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0c906438b05442d66255203eb816453b7193a6d8 Gerrit-Change-Number: 10638 Gerrit-PatchSet: 3 Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180829/d3742351/attachment.htm>