Attention is currently required from: arehbein, 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 3: (5 comments) File include/osmocom/netif/stream.h: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8b8466fb_ad4fb8e2 PS3, Line 48: struct osmo_stream_srv *osmo_stream_srv_create_iofd(void *ctx, const char *name, struct osmo_stream_srv_link *link, int fd, int (*read_cb)(struct osmo_stream_srv *conn, struct msgb *msg), int (*closed_cb)(struct osmo_stream_srv *conn), void *data); Does that mean use needs to use one or another depending on whether iofd is used? Probably that's not the case, so this one should be named osmo_stream_srv_create2. Maybe add extra APIs to set each callback instead. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8b731669_a00dfe5c PS3, Line 83: void osmo_stream_cli_set_iofd_read_cb(struct osmo_stream_cli *cli, int (*read_cb)(struct osmo_stream_cli *cli, struct msgb *msg)); So for client you add a new setter API, but for srv you update an existing one? This looks strange. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/41d831e8_bf7a46b1 PS3, Line 88: struct osmo_stream_cli *osmo_stream_cli_create_iofd(void *ctx, const char *name); osmo_stream_cli_create2 (no iofd specific) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8c683eb8_5dbf7ef4 PS3, Line 1360: #define OSMO_STREAM_SRV_F_FLUSH_DESTROY (1 << 0) Why is this code block moved from below to to here? You are mixing srv_link and srv code then. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/96c519ea_a21ecbcd PS3, Line 1610: rc = conn->read_cb(conn); Mayeb the read_cb rename can be done in a separate prior patch to simplify this one. -- 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: 3 Gerrit-Owner: arehbein <arehbein@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: daniel <dwillmann@sysmocom.de> Gerrit-CC: laforge <laforge@osmocom.org> Gerrit-CC: pespin <pespin@sysmocom.de> Gerrit-Attention: arehbein <arehbein@sysmocom.de> Gerrit-Attention: daniel <dwillmann@sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 19:05:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment