Attention is currently required from: arehbein, laforge, pespin.
1 comment:
Patchset:
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 change 33193. To unsubscribe, or for help writing mail filters, visit settings.