Attention is currently required from: daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30934 )
Change subject: Add osmo_io with initial poll backend ......................................................................
Patch Set 3:
(10 comments)
File src/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/9bae02a3_ce85f35f PS3, Line 389: g_backend == OSMO_IO_BACKEND_POLL ? "poll" : "uring" there might be more in the future, it might be worth having a separate value_string for that rather than explicitly hard-coding the names on the caller side here. Also, it doesn't hurt to log the actaul fd number while we're logging something anyway.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/a750a7b0_dc9a555c PS3, Line 406: / c++ style comment
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/c23a484a_4615b9f0 PS3, Line 430: osmo_iofd_get_priv please rename to osmo_iofd_get_priv_nr(). "priv" is also sometimes used as alias to "data" in kernel and osmocom.
If we remove the argument from the setup function, then we also need a set_priv_nr().
File src/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/221f9c34_279a479c PS3, Line 17: in I'm wondering if it had any advantage to have only a single function with an argument stating the "operation"? The prototype is almost identical (just int/void return).
I'm not sure it would have any advantage. Probably none.
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/4851bdb7_ac9ffd4a PS3, Line 72: struct { : bool read_enabled; : bool read_pending; : bool write_pending; : bool write_enabled; : /* TODO: index into array of registered fd's? */ : } uring; should this maybe introduced only in the actual io_oring patch?
File src/osmo_io_poll.c:
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/620c95b7_ad90f478 PS3, Line 84: / maybe still look into this before merging it?
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/26ed1231_29e5d8c9 PS3, Line 114: extraneous empty line
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/85d8716d_adce74ab PS3, Line 117: { coding style
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/d4b0e7dc_8388e60e PS3, Line 143: / same as above
https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/13c1983d_94d3470c PS3, Line 146: i should be static? same as all of the iofd_poll_ functions which are exposed as callback via iofd_poll_ops and don't need to be exported as global functions