Attention is currently required from: jolly.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34530?usp=email )
Change subject: ASCI: Add ASCI notification support to RR layer ......................................................................
Patch Set 3: Code-Review-1
(10 comments)
File src/host/layer23/src/mobile/gsm48_rr.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/7ad01536_86ad894e PS3, Line 1331: uint8_t *gcr `const`
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/56f2f0ee_e66ae20c PS3, Line 1387: uint8_t *gcr `const`
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/7348a4ae_c9178028 PS3, Line 1399: uint8_t *gcr, uint8_t *ch_desc both `const`
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/1bf90897_7b7b4b41 PS3, Line 1440: return -ENOMEM; You may be leaking the memory allocated by `asci_notif_alloc()` here. Maybe just use `OSMO_ASSERT(nmsg != NULL)`? NULL is unlikely anyway...
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/fc1eebf2_4e6f1fe2 PS3, Line 1461: 5*8 The comment states 36 bits, but you allocate (5 * 8) bytes? I guess you need 5 bytes here, or just `OSMO_BYTES_FOR_BITS(36)`.
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/37c943f4_10f54735 PS3, Line 1467: 3*8 Same here, `OSMO_BYTES_FOR_BITS(24)`, because:
``` \param[in] size Number of bytes in the vector ```
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/f2581c45_059f561f PS3, Line 1473: \n This is going to break the logstring formatting, better use just one `\n`.
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/87f6829b_24176fad PS3, Line 1482: if (chd_bv) `bitvec_free()` is NULL safe, so these checks are redundant.
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/596ddc34_3dea3de9 PS3, Line 1494: msgb_l3len(msg) - sizeof(*sgh); Is it guaranteed by the caller that `msgb_l3len(msg)` is always `>= sizeof(*sgh)`? If not, we may end up with a negative value here and thus huge bitvec length. AFAICS, it's not (see my comments to 34529).
https://gerrit.osmocom.org/c/osmocom-bb/+/34530/comment/5e22edaa_c1637cdf PS3, Line 1519: msgb_l3len(msg) - sizeof(*nn); Same here.