1 Oct
2022
1 Oct
'22
4:07 p.m.
Attention is currently required from: neels, fixeria, pespin.
msuraev has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/28993 )
Change subject: Update multiaddr helper
......................................................................
Patch Set 9:
(6 comments)
File src/socket.c:
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/4f88114f_3d679a33
PS9, Line 206: buf_len
There is no 'buf_len' anymore. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/1bbaa60a_5f2764e9
PS9, Line 211: const char **hosts, size_t host_count,
cosmetic: alignment needs to be fixed here
Done
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/cabdfc50_85dde7a2
PS9, Line 220:
I think it makes sense to assert() here that either
**hosts or **addrs is passed, not both at the sa […]
Seems rather pointless to me:
that's a static function with only few invocations, not part of public API so we
don't have to insure against user who failed to read the docs.
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/ee49f2e0_1692bab0
PS9, Line 236: if (hosts[i])
Before your patch this function would print
"0.0.0.0" if hosts[i] is NULL. […]
That seems like odd side-effect to me
for which I couldn't find any tests. Having NULL input we can't guess whether IPv4
or IPv6 address to show. Moreover, print helper seems like a wrong place for such
guesswork in a first place - the caller have more context information and can pass proper
default explicitly. I'll update commit message to make it more visible.
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/610e45fc_467ccb3f
PS9, Line 241: LOGL_DEBUG
LOGL_ERROR? Not sure if we need to log anything here
though.
Done
https://gerrit.osmocom.org/c/libosmocore/+/28993/comment/4c36e3d1_1f51e592
PS9, Line 749: 512
Better do sizeof(strbuf).
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28993
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Icef53fe4b6e51563d97a1bc48001d67679b3b6e9
Gerrit-Change-Number: 28993
Gerrit-PatchSet: 9
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 01 Oct 2022 16:07:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment