pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31808 )

Change subject: select: Optimize osmo_fd_get_by_fd ......................................................................

Patch Set 1:

(1 comment)

File src/core/select.c:

https://gerrit.osmocom.org/c/libosmocore/+/31808/comment/cd9f53ae_52bfc27f PS1, 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.