neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/18740 )
Change subject: msc_ho: fix CALL IDENTIFIER in HANDOVER REQUEST ...................................................................... Patch Set 3: (3 comments) It looks like my code is not flexible enough to allow handover for non-voice connections: I see that cc_trans is expected to always exist in some places. Nevertheless, please let's not continue on that mistake of mine: call_id should have optional presence everywhere. We should probably do something like .call_id_present = (cc.active_trans != NULL) (I guess we should also add MSC ttcn3 tests to make sure handover on a non-CC conn doesn't crash the MSC! ... just added https://osmocom.org/issues/4612 ) https://gerrit.osmocom.org/c/osmo-msc/+/18740/2/src/libmsc/msc_ho.c File src/libmsc/msc_ho.c: https://gerrit.osmocom.org/c/osmo-msc/+/18740/2/src/libmsc/msc_ho.c@406 PS2, Line 406: .call_id = cc_trans->callref, > Thats indeed not required. The generator function unconditionally picks it, > which is ok. […] I'm pretty sure that's not ok, call_id must be treated optional in all layers of generating a handover request https://gerrit.osmocom.org/c/osmo-msc/+/18740/3/src/libmsc/msc_ho.c File src/libmsc/msc_ho.c: https://gerrit.osmocom.org/c/osmo-msc/+/18740/3/src/libmsc/msc_ho.c@406 PS3, Line 406: .call_id = cc_trans->callref, call_id_present = true https://gerrit.osmocom.org/c/osmo-msc/+/18740/3/src/libmsc/ran_msg_a.c File src/libmsc/ran_msg_a.c: https://gerrit.osmocom.org/c/osmo-msc/+/18740/3/src/libmsc/ran_msg_a.c@1120 PS3, Line 1120: .call_id_present = true, nice catch! but handover may also happen for non-voice connections, so this has to be .call_id_present = n->call_id_present. (I hope n->call_id_present exists?) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/18740 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I6b1b55b3f5a3092d9557dc2512020c766a9ff744 Gerrit-Change-Number: 18740 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Sun, 14 Jun 2020 12:22:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: dexter <pma...@sysmocom.de> Comment-In-Reply-To: pespin <pes...@sysmocom.de> Gerrit-MessageType: comment
