Attention is currently required from: arehbein, laforge, daniel. pespin 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: (8 comments) Patchset: PS5: > > > Since we already have osmo_stream_*_get_ofd() we should add > > > osmo_stream_*_get_iofd(). […] I see no problem with having separate APIs regarding what you mention. The socket may be already enabled, but it may not trigger any callback until you return to the mainloop, so you can perfectly call the APIs to set the callbacks before they are called. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/a95e90f8_70ea30f0 PS7, Line 554: static void handle_connecting(struct osmo_stream_cli *cli, int res) this is for osmo_stream_cli, so adding a "stream_cli" prefix would help easily gastp that. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/a446295c_14b4e708 PS7, Line 598: handle_connecting(cli, res); This split into a separate function you could do it in a previous separate preparation patch. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/078ba248_8761d154 PS7, Line 620: if (msg && res <= 0) { EAGAIN would trigger closing the socket? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/591fa510_4e5313db PS7, Line 881: cli->ofd.priv_nr = 0; /* XXX */ what about this XXX? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/95a2a986_31e7cdd9 PS7, Line 1046: cli->iofd = osmo_iofd_setup(cli, -1, cli->name, OSMO_IO_FD_MODE_READ_WRITE, &osmo_stream_cli_ioops, cli); shouldn't this -1 be "fd"? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/242de708_26f4a629 PS7, Line 1540: osmo_stream_srv_destroy(conn); just wondering whether you may need to return -EBADF here? https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/fa73f28d_48f27e8e PS7, Line 1714: conn->iofd_read_cb = read_cb; See, these can perfectly be separate setter APIs like we do for osmo_stream_cli. -- 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: daniel <dwillm...@sysmocom.de> Gerrit-Comment-Date: Fri, 09 Jun 2023 12:41:28 +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
