Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/12364 )
Change subject: fix ipa_asp_fsm down state transition ...................................................................... Patch Set 1: (3 comments) https://gerrit.osmocom.org/#/c/12364/1/src/xua_asp_fsm.c File src/xua_asp_fsm.c: https://gerrit.osmocom.org/#/c/12364/1/src/xua_asp_fsm.c@920 PS1, Line 920: { > So the only use of fi is to get *priv? Than better just pass struct > ipa_asp_fsm_priv *iafp as parame […] Hmm, yes, good suggestion. https://gerrit.osmocom.org/#/c/12364/1/src/xua_asp_fsm.c@939 PS1, Line 939: osmo_timer_del(&iafp->pong_timer); > I don't understand how it works if we bail out before reaching here. […] This timer is actually never scheduled as far as I can see. And indeed it is bogus that we won't delete it if returning early. But note that this bug is part of code I merely moved around, so I'd rather fix it in a separate patch. https://gerrit.osmocom.org/#/c/12364/1/src/xua_asp_fsm.c@1060 PS1, Line 1060: ipa_asp_fsm_del_route(fi); > I'd rather see ipa_asp_fsm_cleanup() moved in place of > ipa_asp_fsm_del_route() instead of keeping it […] In an earlier version of this patch I just called ipa_asp_fsm_cleanup() from ipa_asp_fsm_active(). What made me uncomfortable about that is that it looks like this cleanup handler is supposed to be invoked by the generic FSM code, not from within the FSM itself. It just so happens that we need to run the same code when the FSM terminates and when we transition ACTIVE->DOWN. I don't really care though because both approaches work. Would you prefer to call ipa_asp_fsm_cleanup() from ipa_asp_fsm_active()? -- To view, visit https://gerrit.osmocom.org/12364 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8e7bd1c74a4b47080fe32ebe0161c503ead571 Gerrit-Change-Number: 12364 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Sperling <s...@stsp.name> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <msur...@sysmocom.de> Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Stefan Sperling <s...@stsp.name> Gerrit-Comment-Date: Thu, 20 Dec 2018 11:42:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
