Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12199 )
Change subject: gsm29118: add generator functions for GSM29118 messages ...................................................................... Patch Set 2: Code-Review-1 (3 comments) https://gerrit.osmocom.org/#/c/12199/2/src/gsm/gsm29118.c File src/gsm/gsm29118.c: https://gerrit.osmocom.org/#/c/12199/2/src/gsm/gsm29118.c@207 PS2, Line 207: len = SGS_MME_NAME_LEN; setting a a local value then returning -1? That makes no sense. https://gerrit.osmocom.org/#/c/12199/2/src/gsm/gsm29118.c@222 PS2, Line 222: msgb_tlv_put(msg, SGSAP_IE_IMSI, len - 2, buf + 2); why not using msgb_put() if we already have the whole TLV in buf? because the TAG is different? https://gerrit.osmocom.org/#/c/12199/2/src/gsm/gsm29118.c@254 PS2, Line 254: struct msgb *gsm29118_create_dl_ud(const char *imsi, struct msgb *nas_msg) does it make sense to require a msgb here API-wise? probably a pointer + len makes more sense. This way if caller doesn't have a msgb doesn't need to create an extra one. -- To view, visit https://gerrit.osmocom.org/12199 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic87f8a771b87b52215d0a7451b67794557b80b8a Gerrit-Change-Number: 12199 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Fri, 07 Dec 2018 17:19:38 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
