librouter: move sendfd, recvfd out of public API #2375
Conversation
Problem: flux_msg_sendfd() et al, and the "iobuf" struct are low level tools for transporting flux messages over file descriptors that have very rough API's and do not need to be included in the public Flux API. As a first step, duplicate flux_msg_sendfd() et al from libflux to the librouter internal library and rename: flux_msg_sendfd() -> sendfd() flux_msg_recvfd() -> recvfd() struct flux_msg_iobuf -> struct iobuf flux_msg_iobuf_init() -> iobuf_init() flux_msg_iobuf_clean() -> iobuf_clean() Duplicate unit tests from libflux/test/message.c. Next steps will be to convert users, then remove public defs.
Problem: sendfd() and recvfd() segfault if presented with invalid arguments. Bad args are detected, but teardown attempts to free storage in uninitialized internal iobuf. Fix the code to return immediately rather than executing the teardown.
Use BAIL_OUT() to handle errors in stuff not being tested, so that ok() output shows actual test results. Add unit tests for sendfd/recvfd that send and receive many messages over a non-blocking pipe, so that the iobuf partial message buffering has coverage. Add coverage for invalid args.
Drop flux_msg_sendfd/recvfd() and flux_msg_iobuf from the public API now that functionality has been replicated in the internal librouter library. Drop unit tests that have also been replicated.
Now that flux_msg_senfd() has been privatized in librouter, drop its public man page.
garlick
force-pushed
the
librouter_sendfd
branch
from
7d53266
to
81b1f27
Compare
|
Rebased on current master, and noting missing coverage of bad args, added some to the unit tests and discovered a segfault, which was fixed. |
|
Restarted a clang builder that failed here (I assume just a spurious timeout) |
|
Hmm, builds finished successfully, but travis-ci completion didn't seem to get picked up by github. I went ahead and restarted the whole travis-ci build. |
Problem: if malloc fails in recvfd(), an errno of EPROTO is used. Don't disturb errno value of ENOMEM already set by malloc() on failure.
Ensure that recvfd() fails with ECONNRESET when read(2) internally returns EOF, as advertised.
Codecov Report
@@ Coverage Diff @@
## master #2375 +/- ##
=========================================
Coverage ? 80.85%
=========================================
Files ? 220
Lines ? 34594
Branches ? 0
=========================================
Hits ? 27971
Misses ? 6623
Partials ? 0
|
|
LGTM! My only comment is that the naming of |
|
Yeah, I wasn't sure about that, but I also didn't have a great alternate name on the tip of my tongue. It isn't published, and it won't have broad usage within flux-core, so could be renamed later if we found a problem. This is prob OK to go in if you think it looks OK. |
|
Thanks! |
Another bit peeled off of #2303 (librouter). This one removes some specialized functions for sending and receiving flux messages over file descriptors from the main API, renaming it and relocating to the private librouter library (which this PR creates). Convert users, add inline documentation, and enhance unit testing.