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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Sat, 01 Oct 2022 16:07:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
