Attention is currently required from: daniel.
8 comments:
Patchset:
In general I think we need to think about naming / namespace pollution. I guess the iofd_* functions are non-static as they will end up geting used form the io_uring backend in another file?
Unfortunately, for historical reasons, libosmocore doesn't have a .map file so all the non-static symbols become publicly accessible to applications. And the general rule for (new) library-exported symbols is to use the osmo_ prefix.
So I think the cleanest approach would actually be to introduce a .map file and then only export the osmo_iofd_* functions that way, while keeping the iofd_* non-static but not exported?
File src/osmo_io.c:
not critical: I usually put the end-of-comment on the previous line to save extraneous lines in the file (more context on the screen) and to optically have the doxygen-docs directly above the function definition, without a (mostly) empty line.
no doxygen docs, but function is exported
Patch Set #3, Line 224: superfluous
superflous IMHO means that it's not needed. Maybe "extraneous"? "leftover"? "pending"?
coding style
coding style
unsigned int size, unsigned int headroom,
int fd
we have three integer argiuments after each other, and lots of arguments, which makes it easy to have bugs on the caller side. Maybe we should re-order them somehow (or reduce, if possible)? Logically I would say that "ctx, fd" should be the two first arguments, as we usually have the object the call relates to/on as the first argument, prefixed by th ctx.
Patch Set #3, Line 358: unsigned int priv_nr
given that there's only one known legacy user of the priv_nr (libosmo-abis), maybe we remove this from the function prototype fore new API. Those few callers that actually care can still set it in the io_fd that is returned from this function?
To view, visit change 30934. To unsubscribe, or for help writing mail filters, visit settings.