> On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, line 52 > > <https://reviews.apache.org/r/12146/diff/1/?file=312848#file312848line52> > > > > No error checking?
See benh's comment below. > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, line 43 > > <https://reviews.apache.org/r/12146/diff/1/?file=312848#file312848line43> > > > > s/pending/_pending/ ? to avoid shadowing the member variable. N/A now. > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines > > 54-55 > > <https://reviews.apache.org/r/12146/diff/1/?file=312848#file312848line54> > > > > I'm a bit confused. What is the difference between pending and blocked? > > Your comment above #39 seems to suggest pending => blocked? If yes, why do > > we need 'unblocked'? This has been refactored now and this should read a lot easier with the helpers. > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, lines > > 58-59 > > <https://reviews.apache.org/r/12146/diff/1/?file=312848#file312848line58> > > > > Why do you have to do this? Probably needs some comments. Removed the errno preservation here. > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp, line 58 > > <https://reviews.apache.org/r/12146/diff/1/?file=312849#file312849line58> > > > > Why on the heap? LOREM_IPSUM is a std::string so the size must be determined at run-time, did you have another way to create the buffer on the stack? > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp, line 12 > > <https://reviews.apache.org/r/12146/diff/1/?file=312849#file312849line12> > > > > How about subclassing TemporaryDirectoryTest? You can get rid of a lot > > of boiler plate below. That lives inside mesos, are you suggesting creating one for stout tests as well? > On July 2, 2013, 7:05 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp, line 138 > > <https://reviews.apache.org/r/12146/diff/1/?file=312848#file312848line138> > > > > why the cast? length is a size_t and we need an off_t for sendfile on OS X. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12146/#review22674 ----------------------------------------------------------- On June 28, 2013, 3:33 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12146/ > ----------------------------------------------------------- > > (Updated June 28, 2013, 3:33 a.m.) > > > Review request for mesos, Benjamin Hindman, Ian Downes, and Vinod Kone. > > > Bugs: MESOS-508 > https://issues.apache.org/jira/browse/MESOS-508 > > > Repository: mesos > > > Description > ------- > > This is to resolve MESOS-508 by masking SIGPIPE per-thread through a > "Suppressor" abstraction. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am > ade668846135bae9e977b65ce0ad0b72865cbf7a > 3rdparty/libprocess/3rdparty/stout/Makefile.am > 864b30ea5661cc7f613fdd422fe55f76fdafd1c1 > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 429039644832470f7fd2eac19b213905cc81dcd3 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/12146/diff/ > > > Testing > ------- > > ./3rdparty/libprocess/3rdparty/stout-tests > --gtest_filter="OsSendfileTest.sendfile" --gtest_repeat=30000 > --gtest_break_on_failure > > > Thanks, > > Ben Mahler > >
