Change in osmo-bsc[master]: Implement support for receiving BSSMAP CommonID from MSC

[neels]

neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/18942 )

Change subject: Implement support for receiving BSSMAP CommonID from MSC
......................................................................


Patch Set 1: Code-Review+1

(4 comments)

https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/bsc_subscr_conn_fsm.c 
File src/osmo-bsc/bsc_subscr_conn_fsm.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/bsc_subscr_conn_fsm.c@794 
PS1, Line 794: 		mi_imsi = data;
When reading this code alone, i immediately ask myself:

- could data be NULL?
- what if the mi->type is not an IMSI?

Of course when investigating, the event dispatch code does duly check that mi_imsi->type == GSM_MI_TYPE_IMSI, and always passes a data pointer.
Still would be nice to have an assert or if () for those assumptions here, too (or a comment)
so future readers are not compelled to grep for event dispatching code.


https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/osmo_bsc_bssap.c 
File src/osmo-bsc/osmo_bsc_bssap.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/osmo_bsc_bssap.c@1078 
PS1, Line 1078: 	} else if ((TLVP_VAL(&tp, GSM0808_IE_IMSI)[0] & GSM_MI_TYPE_MASK) != GSM_MI_TYPE_IMSI) {
this is re-implementing part of osmo_mobile_identity_decode(), let's just drop this else-if because we're also doing the mi.type check below anyway...


https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/osmo_bsc_bssap.c@1084 
PS1, Line 1084: mi_imsi.type != GSM_MI_TYPE_IMSI
...here


https://gerrit.osmocom.org/c/osmo-bsc/+/18942/1/src/osmo-bsc/osmo_bsc_bssap.c@1094 
PS1, Line 1094: 
(ws)



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/18942
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I52c43fb940f0db796adf4c0adb2260321c721c39
Gerrit-Change-Number: 18942
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 22 Jun 2020 15:30:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200622/a29669fe/attachment.htm>