Change in libosmocore[master]: Add osmo_io with initial poll backend - gerrit-log

20 Jan 2023

Attention is currently required from: osmith, Hoernchen, neels, pespin, fixeria, daniel.
laforge 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:

(6 comments)

File src/osmo_io.c:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/6d24eba8_18c9e1a3 
PS3, Line 356: unsigned int size, unsigned int headroom,
             : 		  int fd
...
  I guess we could even have defaults for msgb
size/headroom and have osmo_iofd_set_alloc_info() avail […] yes, we could go for
that.

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c2d9f460_4318f3f3 
PS3, Line 414: 		talloc_free(iofd);
...
  The close callback return value indicates whether the
free() should be deferred or not, it does not  […] I think Pau's comment was
related to the ops.close == NULL case, where you don't free the iofd at all.  So I
guess talloc_free should be called if (close == NULL || (close && close(iofd))

File src/osmo_io_internal.h:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/ea5723dc_ec22fb88 
PS3, Line 87: 	// TODO: SCTP_*
...
  I wanted to keep this patch as-is and add sctp support
in a separate one. […] Ack

File src/osmo_io_poll.c:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/3c922bdc_5f9c8cd9 
PS3, Line 40: what
...
  In socket.c it's called what in the callback, when
in the ofd and flags in the internal code. […] Ack

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/06a0b455_8e16eca4 
PS3, Line 71: 			rc = write(ofd->fd, msgb_data(msg), msgb_length(msg));
...
  There is no iofd->flags, did you mean
msghdr->flags? […] the normal "read/write" case might be worth a
dedicated function as we don't need a serialized msgheader heap allocation for those,
right?

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/0f9dbbd6_88b2d852 
PS3, Line 104: 		socklen_t addrlen = sizeof(struct sockaddr);
...
  I'd use sizeof(saddr) since that most accurately
describes the size recvfrom is allowed to write to. 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: 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: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: fixeria &lt;vyanitskiy(a)sysmocom.de&gt;
Gerrit-Attention: daniel &lt;dwillmann(a)sysmocom.de&gt;
Gerrit-Comment-Date: Fri, 20 Jan 2023 16:35:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith &lt;osmith(a)sysmocom.de&gt;
Comment-In-Reply-To: laforge &lt;laforge(a)osmocom.org&gt;
Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;
Comment-In-Reply-To: daniel &lt;dwillmann(a)sysmocom.de&gt;
Gerrit-MessageType: comment