Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Neels Hofmeyr has submitted this change and it was merged. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. tweak comment to indicate sub_pres_vlr fsm as dead code sub_pres_vlr_fsm_start() only ever has an effect if ms_not_reachable_flag == true. But there simply is no code that sets this flag. So sub_pres_vlr_fsm_start() is currently dead code. Also, examining the FSM, if it should ever be set to true, this would halt the LU/CM Service/Paging response, since the FSM would merely change its state without dispatching asynchronous messages. No chance of finishing. Short of dropping the code entirely, first just mark it. The point being that this models some FSM definition from 3GPP specs, and we have a couple other "if (0)" branches in the VLR... Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 --- M src/libvlr/vlr_lu_fsm.c 1 file changed, 3 insertions(+), 1 deletion(-) Approvals: Neels Hofmeyr: Looks good to me, approved Stefan Sperling: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c index 74a6939..a0cbcab 100644 --- a/src/libvlr/vlr_lu_fsm.c +++ b/src/libvlr/vlr_lu_fsm.c @@ -271,7 +271,9 @@ return (struct vlr_subscr*)fi->priv; } -/* Note that the start event is dispatched right away, so in case the FSM immediately concludes from that +/* THIS IS CURRENTLY DEAD CODE, SINCE WE NEVER SET vsub->ms_not_reachable_flag = true. + * + * Note that the start event is dispatched right away, so in case the FSM immediately concludes from that * event, the created FSM struct may no longer be valid as it already deallocated again, and it may * furthermore already have invoked the parent FSM instance's deallocation as well. Hence, instead of * returning, store the created FSM instance address in *fi_p before dispatching the event. It is thus -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 4 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling Gerrit-CC: Max Gerrit-CC: Pau Espin Pedrol
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. Patch Set 3: Code-Review+2 pulling the "comment / triviality" rule -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 3 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling Gerrit-CC: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 17 Dec 2018 14:39:08 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/12234/3/src/libvlr/vlr_lu_fsm.c File src/libvlr/vlr_lu_fsm.c: https://gerrit.osmocom.org/#/c/12234/3/src/libvlr/vlr_lu_fsm.c@182 PS3, Line 182: * Subscriber_Present_VLR, TS 29.002 Chapter 25.10.1 the spec reference is here, max, similar to all the libvlr FSMs -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 3 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling Gerrit-CC: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Mon, 17 Dec 2018 13:03:46 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Max has posted comments on this change. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/12234/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/12234/3//COMMIT_MSG@18 PS3, Line 18: this models some FSM definition from 3GPP specs, and we have a couple other It would be better to add actual spec reference here as well as in the comment. -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 3 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Stefan Sperling Gerrit-CC: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Fri, 14 Dec 2018 12:57:24 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/#/c/12234/2/src/libvlr/vlr_lu_fsm.c File src/libvlr/vlr_lu_fsm.c: https://gerrit.osmocom.org/#/c/12234/2/src/libvlr/vlr_lu_fsm.c@274 PS2, Line 274: /* THIS IS CURRENTLY DEAD CODE, SINCE WE NEVER SET vsub->ms_not_reachable_flag = true. Probably also good to state the same in the callers. -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 2 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Stefan Sperling Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Tue, 11 Dec 2018 11:38:26 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/12234 ) Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 2 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Stefan Sperling Gerrit-Comment-Date: Tue, 11 Dec 2018 09:45:33 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: tweak comment to indicate sub_pres_vlr fsm as dead code
Neels Hofmeyr has uploaded this change for review. ( https://gerrit.osmocom.org/12234 Change subject: tweak comment to indicate sub_pres_vlr fsm as dead code .. tweak comment to indicate sub_pres_vlr fsm as dead code sub_pres_vlr_fsm_start() only ever has an effect if ms_not_reachable_flag == true. But there simply is no code that sets this flag. So sub_pres_vlr_fsm_start() is currently dead code. Also, examining the FSM, if it should ever be set to true, this would halt the LU/CM Service/Paging response, since the FSM would merely change its state without dispatching asynchronous messages. No chance of finishing. Short of dropping the code entirely, first just mark it. The point being that this models some FSM definition from 3GPP specs, and we have a couple other "if (0)" branches in the VLR... Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 --- M src/libvlr/vlr_lu_fsm.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/34/12234/1 diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c index 74a6939..a0cbcab 100644 --- a/src/libvlr/vlr_lu_fsm.c +++ b/src/libvlr/vlr_lu_fsm.c @@ -271,7 +271,9 @@ return (struct vlr_subscr*)fi->priv; } -/* Note that the start event is dispatched right away, so in case the FSM immediately concludes from that +/* THIS IS CURRENTLY DEAD CODE, SINCE WE NEVER SET vsub->ms_not_reachable_flag = true. + * + * Note that the start event is dispatched right away, so in case the FSM immediately concludes from that * event, the created FSM struct may no longer be valid as it already deallocated again, and it may * furthermore already have invoked the parent FSM instance's deallocation as well. Hence, instead of * returning, store the created FSM instance address in *fi_p before dispatching the event. It is thus -- To view, visit https://gerrit.osmocom.org/12234 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I198d442e9ed288f37c7d4e5ec87b82dc53114e99 Gerrit-Change-Number: 12234 Gerrit-PatchSet: 1 Gerrit-Owner: Neels Hofmeyr
