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. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31808 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade Gerrit-Change-Number: 31808 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 09 Mar 2023 17:50:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
