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 <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)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 <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment