View Change

1 comment:

• File src/core/select.c:

  • Patch Set #1, Line 223: if (fd->fd >= 0)

    Here I'd like to have an OSMO_ASSERT(fd->fd >=0) instead, but that made one unit test in osmo-mgw hit the assert: https://gerrit.osmocom.org/c/osmo-mgw/+/31806
    Other projects are running fine so far (A previous patch for libosmocore also fixes one related issue heating this assert).

    Even if that makes older osmo-mgw assert, I think it is still acceptable because it is actually detecting a bug in osmo-mgw.
    Users of the osmo_fd API shouldn't change the fd value while the osmo_fd is registered, that's calling for a potential mess.
    This actually even causes bugs inside libosmocore before this patch, see call to osmo_stats_tcp_osmo_fd_unregister(fd) just below, which will probably leak due to not finding the fd because it has been set to -1 prior to unregistering.

    So, do others agree I should use an OSMO_ASSERT there? this will help in finding bugs during development time by hitting the assert, rather than ending up with fucked up silently at production time.

To view, visit change 31808. To unsubscribe, or for help writing mail filters, visit settings.