Attention is currently required from: pespin. falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmocore/+/39276?usp=email ) Change subject: osmo_iofd_register: fix the case of changing fd ...................................................................... Patch Set 1: (6 comments) Patchset: PS1: I am working on OS#6474. I got this code: https://gitea.osmocom.org/themwi/twrtp-proto which I am currently reworking into a series of patches for libosmo-netif, with the goal of bringing my new RTP implementation into Osmocom proper and thus making it available to OsmoBTS and others in the future. The version which I plan to submit as patches to libosmo-netif will be a little different from the just-linked external/standalone version: aside from renaming headers and APIs to fit into Osmocom namespace, I am reworking the core structures (`struct twrtp_jibuf_inst` which will become `struct osmo_twjit` and `struct twrtp_endp` which will become `struct osmo_twrtp`) to be opaque instead of fully exposed - hence I need to provide APIs for all explicitly-allowed external operations. In the new version there will be an API that takes in OS-level fds for RTP and RTCP sockets and feeds them to `osmo_iofd_register()` on `osmo_io_fd` instances that are internal to the opaque twrtp instance. The problem is - what happens if one of those `osmo_iofd_register()` operations fails? My twrtp API needs to fail cleanly, and leave the twrtp instance in a state where the user could try again and potentially succeed next time. I reviewed osmo_io code to see if it does what I expect, and noticed that if `osmo_iofd_register()` fails, the failed fd will remain stored, and the next `osmo_iofd_register()` attempt will ignore the fd passed to it. Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/39276/comment/0926db90_88079472?… : PS1, Line 13: osmo_fd_setup(), or if it has changed since then, you can state Acknowledged https://gerrit.osmocom.org/c/libosmocore/+/39276/comment/96f838a1_3c01affc?… : PS1, Line 22: OK, I'll add this notation in the next iteration. File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/39276/comment/5f8b3498_bb0eb7e5?… : PS1, Line 756: else if (iofd->fd < 0) Right after the `else if` clause I removed, the code checks `IOFD_FLAG_FD_REGISTERED`. How can `iofd->fd` be -1 if the registered flag is set? And if the registered flag is not set, then my code sets `iofd->fd` to the new fd unconditionally. But maybe I'll leave this code unchanged in the next iteration - let me think about it. https://gerrit.osmocom.org/c/libosmocore/+/39276/comment/e7c591f3_e3a32f94?… : PS1, Line 760: return iofd->fd == fd ? 0 : -ENOTSUP; See my other comment above. https://gerrit.osmocom.org/c/libosmocore/+/39276/comment/7e3efd03_e4238e8b?… : PS1, Line 761: } else { Acknowledged -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/39276?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: If8b8486ad7934afa203dfe1e996c9217373a017b Gerrit-Change-Number: 39276 Gerrit-PatchSet: 1 Gerrit-Owner: falconia &lt;falcon(a)freecalypso.org&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Comment-Date: Thu, 09 Jan 2025 20:34:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;