laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/22046 )
Change subject: gprs_bssgp: add utilities to send and parse BSSGP rim PDUs ...................................................................... Patch Set 6: (2 comments) https://gerrit.osmocom.org/c/libosmocore/+/22046/6/include/osmocom/gprs/gprs_bssgp.h File include/osmocom/gprs/gprs_bssgp.h: https://gerrit.osmocom.org/c/libosmocore/+/22046/6/include/osmocom/gprs/gprs_bssgp.h@73 PS6, Line 73: in those two messages are not symmetric. One is encoding + transmitting, while the other is jsut decoding. I would argue we want at least an encode + decode function, while the transmit is more like an optional (but welcome additional) helper. So something like 'struct msgb *bssgp_encode_rim_pdu(const struct bssgp_ran_informatoin_pdu *)' is missing. The bssgp_tx_rim() can then simply call that function, set msgb_nsei and call bssgp_ns_send(). https://gerrit.osmocom.org/c/libosmocore/+/22046/6/src/gb/gprs_bssgp_util.c File src/gb/gprs_bssgp_util.c: https://gerrit.osmocom.org/c/libosmocore/+/22046/6/src/gb/gprs_bssgp_util.c@701 PS6, Line 701: rc = why is this re-inventing a simplistic BSSGP message parser? We do have a "proper" BSSGP parser whihc validates the mandatory IEs and also validates the minimum length of all IEs: osmo_tlv_prot_parse(&osmo_pdef_bssgp, tp, ARAY_SIZE(tp), bgph->data, data_len, 0, 0, DBSSGP, __func__) One of the benefits of using that is that you don't need to check for PRES_LEN everywhere below, but can just use TLVP_PRES, as the minimum length is already ensured by the parser. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/22046 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I18134fd9938040d2facb6beee3732628b167ce8c Gerrit-Change-Number: 22046 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Comment-Date: Tue, 19 Jan 2021 11:17:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
