Attention is currently required from: osmith, Hoernchen, neels, laforge, pespin, fixeria. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 ) Change subject: Add osmo_io with initial poll backend ...................................................................... Patch Set 4: (11 comments) This change is ready for review. Patchset: PS3: Are you talking about a map file for all of libosmocore or just a libosmoio map file? File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8c9b0167_87a37287 PS3, Line 50: struct iofd_backend_ops g_iofd_ops; This should be resolved if we end up introducing a .map file, right? File src/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/cdabab1f_b2c19701 PS4, Line 228: msg_pending = iofd_msgb_alloc(iofd); This function now sets pending_out to the remaining msgb and iofd_handle_segmented_read is responsible for setting this. This also saves us one msgb_pending call in the handle_more case below. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0a9f73e5_028dca84 PS4, Line 299: * \returns 0 in case of success; a negative value in case of error Even though almost no code uses it I guess you're right. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1115e0e0_632590fc PS4, Line 356: : struct os Done https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/b7a0d6ae_03597afb PS4, Line 367: iofd->io_ops = *ioops; I agree, the name should be mandatory. https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8471d7a4_6109cd5a PS4, Line 414: * \param[in] headroom the headroom of the msgb when receiving data Ack File src/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/8e8e3792_75bbe29d PS3, Line 17: in I'll keep it as it is https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/337a22b0_b3a229c7 PS3, Line 72: struct { : bool read_enabled; : bool read_pending; : bool write_pending; : bool write_enabled; : /* TODO: index into array of registered fd's? */ : } uring; Done File src/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/66e76cf0_7c9f532a PS4, Line 104: struct osmo_sockaddr saddr; Done https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f3a3dc66_965873ff PS4, Line 128: if (rc < 0) { Does the kernel use addrlen for more than just checking if it's long enough for the variant? In any case I guess we only want to copy to the kernel what we really need. I think should be a helper function in libosmocore like osmo_sockaddr_size() that returns the size depending on the family that way we can reuse it elsewhere. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0 Gerrit-Change-Number: 30934 Gerrit-PatchSet: 4 Gerrit-Owner: daniel &lt;dwillmann(a)sysmocom.de&gt; Gerrit-Reviewer: Hoernchen &lt;ewild(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Attention: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Attention: Hoernchen &lt;ewild(a)sysmocom.de&gt; Gerrit-Attention: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Attention: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Comment-Date: Wed, 25 Jan 2023 16:03:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge &lt;laforge(a)osmocom.org&gt; Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: comment