Attention is currently required from: jtavares. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-remsim/+/30139 )
Change subject: bankd: Add GSMTAP functionality for SIM traffic ......................................................................
Patch Set 2:
(5 comments)
Patchset:
PS2: In general I'd like to see this feature included, just some minor stuff, see comments.
File src/bankd/gsmtap.c:
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/5090e9b3_1e1d2f79 PS2, Line 28: osmo_bankd_gsmtap_init there's a problem with the naming prefix, sorry. In general, we reserve the osmo_ prefix of global/exported symbols to our libraries like libosmo{core,abis,gsm,netif,sccp,sigtran,mgcp,...}. Applications should generally not use this prefix to avoid potential future namespace collissions with library functions introduced at a later point.
See also https://osmocom.org/projects/cellular-infrastructure/wiki/Coding_standards#N...
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/07aac0ee_c5a3ea69 PS2, Line 35: perror("unable to open GSMTAP"); normally we'd want to use the libosmocore logging framework with LOGL_ERROR here.
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/c387ce7e_dc36944b PS2, Line 56: uint8_t *buf = malloc(gross_len); not critical, just a comment: we could do without a dynamic heap allocation + memcpy here if we were to use writev() instad of write(). This way we could have the header on the stack, and an iovec[3] for header, mdm-tpdu and sim-tpdu.
It's not critical as this is not a performance critical code path.
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/e54d931c_c9643199 PS2, Line 83: perr normally we'd want to use the libosmocore logging framework with LOGL_ERROR here.