Attention is currently required from: fixeria, laforge, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/35287?usp=email )
Change subject: stream_srv_link: osmo_stream_srv_link_get_sockname() now returns the full set of addresses ...................................................................... Patch Set 3: (4 comments) File src/stream_srv.c: https://gerrit.osmocom.org/c/libosmo-netif/+/35287/comment/f5d0d3b2_5dff3ce7 PS3, Line 89: char sockname[OSMO_STREAM_MAX_ADDRS * INET6_ADDRSTRLEN + OSMO_STREAM_MAX_ADDRS + 2 + 6 + 1]; > I'm not sure it's worth it, it's not like you are using it everywhere, and > it's really related to th […] I ended up adding it here: https://gerrit.osmocom.org/c/libosmocore/+/35330 socket: Introduce defines OSMO_SOCK_MULTIADDR_{PEER_STR,NAME}_MAXLEN I will rework these commits once that one is merged. https://gerrit.osmocom.org/c/libosmo-netif/+/35287/comment/5cf1a9af_af644580 PS3, Line 364: "<need-more-bufs!>" > This string (or some part of it) may not make it to the output buffer if > there is not enough room in […] * result mixed with an error message: better than return silently a truncated output. * may not even fit: need_more_bufs is printed in the event not enough buffers were provided to obtain the list of addresses, not if the output buffer is not long enough. IIRC right now the kernel has a limit of 32 addresses at SCTP level, so in general we are save here. But in the event something is really wrong in really big setups, better print this. https://gerrit.osmocom.org/c/libosmo-netif/+/35287/comment/6395e0dd_df5056b2 PS3, Line 371: NULL in case of error > this is no longer true the fact that the implementation internally doesn't fail right now doesn't mean I should change the API. I think it's good to keep users checking for errors in APIs which may change implementations, like this one. https://gerrit.osmocom.org/c/libosmo-netif/+/35287/comment/7b9a158e_74d821bc PS3, Line 467: get_local_sockname_buf > Do we want to check return value against NULL here? […] what would you suggest to do instead? I don't see anything interesting/worth here. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/35287?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I216502a9aeafe638940f110bc9fddf2504b2ac3a Gerrit-Change-Number: 35287 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: osmith <osm...@sysmocom.de> Gerrit-Attention: osmith <osm...@sysmocom.de> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Attention: fixeria <vyanits...@sysmocom.de> Gerrit-Comment-Date: Tue, 12 Dec 2023 13:08:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osm...@sysmocom.de> Comment-In-Reply-To: pespin <pes...@sysmocom.de> Comment-In-Reply-To: fixeria <vyanits...@sysmocom.de> Gerrit-MessageType: comment
