Attention is currently required from: fixeria, pespin, msuraev.
Patch set 13:Code-Review -1
6 comments:
Commit Message:
Patch Set #13, Line 10: * properly handle OOM
OOM is not the responsibility of functions like snprintf(), the caller shall check rc < len
File src/core/socket.c:
Patch Set #13, Line 229: const struct sockaddr *addr = &(*addrs)[i];
to conform with typical osmo style, suggest using struct osmo_sockaddr instead, and below use addr.u.sas; and osmo_sockaddr_to_str_buf2() (s.b.)
Patch Set #13, Line 244: OSMO_STRBUF_PRINTF(*sb, "%s%s", oss->ip, after);
no need to use osmo_sockaddr_str here, instead do
OSMO_STRBUF_APPEND(sb, osmo_sockaddr_to_str_buf2, &addr);
OSMO_STRBUF_PRINTF(sb, "%s", after);
Patch Set #13, Line 252: return -ENOSPC;
do not size check!! this breaks the osmo_strbuf usage.
A foo_buf() function should always return sb.chars_needed, the nr of characters that *would* have been written if the buffer were long enough, like snprintf() does. this enables the caller to allocate enough buffer and call the function again (which OSMO_NAME_C_IMPL() does). That is the entire idea behind the OSMO_STRBUF_* macros, which never write past the buffer length and ensure terminating nul like snprintf(). Also same as with snprintf(), it is the caller's responsibility to check that the returned len is < the buffer len.
just put the function block of multiaddr_strbuf() in here.
the typical pattern we have is foo_buf() and foo_c() functions, rather not open another family of functions that pass osmo_strbuf around. OSMO_STRBUF_APPEND() is there to just use the foo_buf() variant efficiently and avoid passing osmo_strbuf around. IOW an osmo_strbuf should remain within a function body.
File src/socket.c:
Seems rather pointless to me: that's a static function with only few invocations, not part of public […]
(assert()s also clarify to the reader the intended usage, and to coverity what to expect. so it's a good thing to add, but also not super critical)
To view, visit change 28993. To unsubscribe, or for help writing mail filters, visit settings.