Attention is currently required from: fixeria, laforge, osmith.

View Change

4 comments:

• File src/stream_srv.c:

  • Patch Set #3, 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.

  • Patch Set #3, 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.

  • Patch Set #3, 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.

  • Patch Set #3, 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 change 35287. To unsubscribe, or for help writing mail filters, visit settings.