Attention is currently required from: osmith, Hoernchen, neels, laforge, fixeria, daniel.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend
......................................................................
Patch Set 5:
(6 comments)
File include/osmocom/core/osmo_io.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d9628e2d_f17a91bf
PS5, Line 57: int osmo_iofd_sendto_msgb(struct osmo_io_fd *iofd, struct msgb *msg, int
flags,
maybe name it sendto_flags to clarify what are the flags for, but not that important.
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/ca524f17_c67c82bf
PS5, Line 210: * If there are bytes left over *pending_out will point to a msgb with the
remaining data.
" If there are bytes left over, *pending_out will" (see extra comma)
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/7c6dffd2_0e001c04
PS5, Line 426: if (!osmo_iofd_ops.close || (osmo_iofd_ops.close &&
osmo_iofd_ops.close(iofd)))
There seems to be a lot of expected stuff happening here with regards to struct
configuration and return codes from function pointers.
I think this deserves a comment here explaining the expected lifecycle.
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/1fa54ade_1463fb7d
PS3, Line 71: rc = write(ofd->fd, msgb_data(msg), msgb_length(msg));
the normal "read/write" case might be worth
a dedicated function as we don't need a serialized msghe […]
read/write is
basically recv/send with flags=0, so there's no need to have read/write imho.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/f4a749b6_e40208a1
PS3, Line 84: /
maybe still look into this before merging it?
Ack
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c7a750bf_aea0ba83
PS3, Line 143: /
same as above
Ack
--
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: 5
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Feb 2023 15:29:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment