Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 )
Change subject: hlr.c: forward GSUP messages between clients ...................................................................... Patch Set 13: (3 comments) https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@469 PS13, Line 469: session_id > AFAIR, libosmocore doesn't encode the OSMO_GSUP_SESSION_ID_IE if > gsup_msg->session_state == 0. […] (So far none of the forwarded messages use a session id or session state. For MAP/DIAMETER compatibility, that might be necessary/nice-to-have in the future though) https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@485 PS13, Line 485: if (!msgb_l2(msg) || !msgb_l2len(msg)) > This check is redundant and not actually needed, since we do > osmo_gsup_decode(msgb_l2(msg), msgb_l2l […] I see it as an initial sanity check. If msgb_l2() is NULL, we may run into segfaults, if the len is 0, we should reject it. I'd rather keep this check. This comes from buggy code actually killing osmo-hlr, instead it should merely complain and continue to run. https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@501 PS13, Line 501: LOGP_GSUP_FWD > Here we would see 'OSMO_GSUP_MSGT_E_ROUTING_ERROR' as the message name, > because we don't store the o […] Ok, you are saying the logging fails to log the initial message, and instead logs the Routing Error reply. Nice catch. The problem is that at this point the initial msg has already been discarded... maybe LOG_GSUP_FWD should have a separate arg passing the original message type? -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 13 Gerrit-Owner: osmith <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-CC: Vadim Yanitskiy <[email protected]> Gerrit-Comment-Date: Tue, 07 May 2019 13:38:52 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
