Patch Set 3: (4 comments)
https://gerrit.osmocom.org/#/c/7142/3/src/libbsc/bsc_subscr_conn_fsm.c File src/libbsc/bsc_subscr_conn_fsm.c: Line 246: /* FIXME: Extract Mobile ID and update FSM using osmo_fsm_inst_set_id() */ related: https://osmocom.org/issues/2969, i.e. we will probably extract the mobile identity earlier, where the imsi filter code is. Then we just take the bsub->imsi here or something. Line 265: /* FIXME: Extract optional IMSI and update FSM using osmo_fsm_inst_set_id() */ related: https://osmocom.org/issues/2969 (same as above) Line 322: * should be restructured */ should still be restructured? https://gerrit.osmocom.org/#/c/7142/3/tests/handover/handover_test.c File tests/handover/handover_test.c: Line 63: * Unfortunately, the fi pointer we get here is (normally) unpopulated "normally", the fi is of course populated, just in this artificial test setup it isn't. I think this comment should shout really loudly first off and above all that the fi pointer is misused to referene a completely unrelated instance, and then explain a little bit why. Right now from reading this comment as uninformed reader I don't understand the hack. Added bonus would be to still use the bts_fi like in a real setup and wrap away all its outside actions, so that we don't need super ugly hacks like this. On a different take, @laforge: should we consider taking this entire test suite out of here into ttcn3? Would be some coding effort, but then we'd not have these kinds of issues. -- To view, visit https://gerrit.osmocom.org/7142 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I68286d26e2014048b054f39ef29c35fef420cc97 Gerrit-PatchSet: 3 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-HasComments: Yes
