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
-
msuraev