pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-iuh/+/26722 )
Change subject: iu_helpers: make new_transp_info_(rtp|gtp) public ......................................................................
Patch Set 3:
(8 comments)
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c File src/iu_helpers.c:
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@134 PS3, Line 134: switch (addr->u.sin.sin_family) { you should better use u.sa here conceptually, since you don't really know whether it's AF_INET (u.sin) or not. In practice it doesn't matter since it ends up in same memory position.
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@140 PS3, Line 140: ip_len = sizeof(addr->u.sin6.sin6_addr.s6_addr); Not sure if this is correct or needs to be sizeof(addr->u.sin6.sin6_addr) or it doesn't matter.
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@148 PS3, Line 148: len = 160 / 8; can you explain/document these numbers?
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@183 PS3, Line 183: binding_id[0] = port & 0xff; probably osmo_store16be() would be more clear here.
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@187 PS3, Line 187: new_transp_layer_addr(&tli->transportLayerAddress, addr, use_x213_nsap); check return code an return NULL
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/iu_helpers.c@201 PS3, Line 201: new_transp_layer_addr(&tli->transportLayerAddress, addr, use_x213_nsap); check return code an return NULL
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/ranap_msg_factory.c File src/ranap_msg_factory.c:
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/ranap_msg_factory.c@753 PS3, Line 753: rtp_addr.u.sin.sin_addr.s_addr = htonl(rtp_ip); are you sure the rtp_ip is passed above in host byte order? looks weird to me.
https://gerrit.osmocom.org/c/osmo-iuh/+/26722/3/src/ranap_msg_factory.c@826 PS3, Line 826: gtp_addr.u.sin.sin_addr.s_addr = htonl(gtp_ip); same