> On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/include/process/io.hpp, line 44 > > <https://reviews.apache.org/r/5576/diff/3/?file=116935#file116935line44> > > > > 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'.
Remove readAll as no efficient way to provide transactional semantics. > On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/process.cpp, line 3005 > > <https://reviews.apache.org/r/5576/diff/3/?file=116936#file116936line3005> > > > > Kill space. Done. > On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/process.cpp, line 3023 > > <https://reviews.apache.org/r/5576/diff/3/?file=116936#file116936line3023> > > > > 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. ;) Done. > On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/process.cpp, line 3151 > > <https://reviews.apache.org/r/5576/diff/3/?file=116936#file116936line3151> > > > > Let's implement 'readAll' interms of 'read'. Removed. > On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/tests.cpp, lines 1113-1114 > > <https://reviews.apache.org/r/5576/diff/3/?file=116937#file116937line1113> > > > > For tests, you can just stick stuff like this on one line (see previous > > tests): > > > > ASSERT_NE(-1, pipe(pipes)); Done. > On June 27, 2012, 8:15 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/tests.cpp, line 1133 > > <https://reviews.apache.org/r/5576/diff/3/?file=116937#file116937line1133> > > > > 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)). > > Done. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5576/#review8664 ----------------------------------------------------------- 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 > >
