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).