Attention is currently required from: arehbein, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 )
Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Patch Set 7: (1 comment) Patchset: PS5: > > Since we already have osmo_stream_*_get_ofd() we should add > > osmo_stream_*_get_iofd(). Since they will return pointer we could simply > > return NULL if it's called with the wrong mode, but maybe an assert is > > better. > > The question here would be: do we really need that API for iofd? or iofd can > be handled totally internally? Ok, it seems this is mostly used in the actual read_cb to get the fd to perform the read. Other uses also use the file descriptor with osmo_sock_get_name2(). So I guess we don't need it. > > osmo_stream_srv_create2 is called with the newly accept()ed fd so I'd still > > pass the read/close callbacks in create2(). > > I'd keep setting the callbacks as separate APIs. Yes, it does 2 function > calls more, occupies 2 more lines, but if we had done this already in the > past then now we wouldn't need to be changing this osmo_stream_srv_create() > API in first place... so learning for errors = adding separate setter APIs. Well, it's 3 more - setting the callbacks and then calling a function that actually enables reading from the fd. You usually call osmo_stream_srv_create() inside of the connect_cb of osmo_strem_srv_link. At that point the fd is already connected since it has been accept()ed and might also have pending data. Every osmo_stream_* where the fd is initially "inactive" has separate setter functions for the callbacks so this choice feels deliberate to me (and I agree with it). -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein <arehb...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-CC: daniel <dwillm...@sysmocom.de> Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: arehbein <arehb...@sysmocom.de> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 12:14:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <lafo...@osmocom.org> Comment-In-Reply-To: pespin <pes...@sysmocom.de> Comment-In-Reply-To: daniel <dwillm...@sysmocom.de> Gerrit-MessageType: comment
