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



--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/26722
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I1e369718de8c4c7db1f1af1e6864562164ada6cf
Gerrit-Change-Number: 26722
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Comment-Date: Thu, 06 Jan 2022 20:39:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to