----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5576/#review8664 -----------------------------------------------------------
third_party/libprocess/include/process/io.hpp <https://reviews.apache.org/r/5576/#comment18324> Let's simplify this more: readAll either returns all the data, or an error (no need for 'actual'). Anyone who wants to get "partial" data will just use 'read'. third_party/libprocess/src/process.cpp <https://reviews.apache.org/r/5576/#comment18318> Kill space. third_party/libprocess/src/process.cpp <https://reviews.apache.org/r/5576/#comment18325> I'd love to move this out as a CHECK before line 3017, since these semantics should be guaranteed by our implementation. Move the comment too of course. ;) third_party/libprocess/src/process.cpp <https://reviews.apache.org/r/5576/#comment18326> Let's implement 'readAll' interms of 'read'. third_party/libprocess/src/tests.cpp <https://reviews.apache.org/r/5576/#comment18322> For tests, you can just stick stuff like this on one line (see previous tests): ASSERT_NE(-1, pipe(pipes)); third_party/libprocess/src/tests.cpp <https://reviews.apache.org/r/5576/#comment18323> Let's factor out making a file descriptor blocking into the process::io namespace. This will be incredibly useful beyond just testing! namespace io { bool nonblock(int fd) { ... fcntl(...); } } It would be great to have a way to check if something is non-blocking too (maybe nonblock(fd) and isNonblock(fd)). - Benjamin Hindman On June 27, 2012, 6:44 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5576/ > ----------------------------------------------------------- > > (Updated June 27, 2012, 6:44 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Description > ------- > > Provide an interface in process::io to do non-blocking read. It leverage the > io polling mechanism introduced in https://reviews.apache.org/r/5424/ > > > Diffs > ----- > > third_party/libprocess/include/process/io.hpp 801a6d5 > third_party/libprocess/src/process.cpp b7c181e > third_party/libprocess/src/tests.cpp 3d5005c > > Diff: https://reviews.apache.org/r/5576/diff/ > > > Testing > ------- > > cd /path/to/libprocess/build > ./tests > > Tested on both Mac and Linux. > > > Thanks, > > Jie Yu > >
