pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/22368 )
Change subject: gprs_bssgp_rim: add serving BSS NACC application ...................................................................... Patch Set 6: (3 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/22368/6/src/gprs_bssgp_rim.c File src/gprs_bssgp_rim.c: https://gerrit.osmocom.org/c/osmo-pcu/+/22368/6/src/gprs_bssgp_rim.c@69 PS6, Line 69: app_cont->si[app_cont->num_si] = bts->si1 + 2; Add comment explaining what's this +2 about. https://gerrit.osmocom.org/c/osmo-pcu/+/22368/6/src/gprs_bssgp_rim.c@266 PS6, Line 266: format_response_pdu(&resp_pdu, pdu, bts); You are passing here the "dst BTS" and you are then using then using it to gather the information to fill the response message. However, you should probably instead be looking up the BTS based on CGI-PS from "reprt_cell" from NACC application in higher layers. I agree though that in general both addresses should be the same, but I would then at least add some sanity check making sure both addresses match. https://gerrit.osmocom.org/c/osmo-pcu/+/22368/6/src/pcu_l1_if.cpp File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/22368/6/src/pcu_l1_if.cpp@300 PS6, Line 300: if (len == 1) { switch (len) on all the possibilities in this function. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/22368 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Id72118120c14984d2fb1b918b41fac4868150d41 Gerrit-Change-Number: 22368 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Comment-Date: Mon, 01 Feb 2021 11:03:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
