Attention is currently required from: Hoernchen, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32021 ) Change subject: gmm: Initial implementation of GPRS Attach ...................................................................... Patch Set 1: (11 comments) File include/osmocom/gprs/gmm/gmm_pdu.h: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/bda939af_3011717a PS1, Line 16: tlv_definition You should include `<osmocom/gsm/tlv.h>` defining this struct. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/49f0d57d_a1111060 PS1, Line 32: uint8_t sres[4], Why not a const pointer? File src/gmm/gmm_pdu.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/ce79165d_3ba343e8 PS1, Line 31: struct gprs_gmm_ms_net_cap { Run `struct_endianess.py`. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/e41600ea_e2575114 PS1, Line 142: 23 `GSM_MAC_BLOCK_LEN`? https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/964e63e8_8294d257 PS1, Line 147: rc = bitvec_unhex(&bv, "171933432b37159ef98879cba28c6621e72688b198879c00"); Makes sense to add a TODO here, since you're hard-coding the payload. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/253cfe33_2e5895a3 PS1, Line 172: msgb_put_u8(msg, sizeof(ms_net_cap_def)); Use `msgb_lv_put()`. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/2f6aa280_f970ab21 PS1, Line 274: GSM_MI_TYPE_TMSI `GSM_MI_TYPE_IMEI`! https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/45b97646_44260463 PS1, Line 312: (void)imeisv_requested; What is this for? File src/gmm/gmm_prim.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/6b82593c_86df9176 PS1, Line 107: gmm_llc_down_cb_dummy `__func__` https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/20a3b78f_af6a4735 PS1, Line 467: rc = gprs_gmm_prim_handle_unsupported(gmm_prim); This case can fall-through to default, but not critical. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/f22b7262_9b95b81a PS1, Line 589: llc_prim Why would anybody pass you NULL pointer here? Just curious. -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32021 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I212053b3a3f27ef7d63503c3d5ef08453b2d2056 Gerrit-Change-Number: 32021 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Hoernchen &lt;ewild(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Attention: Hoernchen &lt;ewild(a)sysmocom.de&gt; Gerrit-Attention: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Comment-Date: Wed, 22 Mar 2023 21:59:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment