> On Sept. 13, 2012, 12:13 a.m., Vinod Kone wrote: > > src/files/files.cpp, line 167 > > <https://reviews.apache.org/r/7048/diff/2/?file=152776#file152776line167> > > > > why const here and not for fd or offset?
this is due to a bug in libprocess, snippet from previous review: benh: "Please add the following at the end of this line: // TODO(benh): Remove 'const &' after fixing libprocess." me: "done, lost me on this one..?" benh: "Normally you wouldn't need to make a primitive like size_t be const &, but in this case I only have Future::then versions for arguments that are const &, so that's why it has to be const &. This is a bug on my side." so I've added the same todo here now to clarify > On Sept. 13, 2012, 12:13 a.m., Vinod Kone wrote: > > third_party/libprocess/src/tests.cpp, line 1236 > > <https://reviews.apache.org/r/7048/diff/2/?file=152779#file152779line1236> > > > > CHECK(data.size() == 128) ? :) hahah, alright > On Sept. 13, 2012, 12:13 a.m., Vinod Kone wrote: > > src/files/files.cpp, line 301 > > <https://reviews.apache.org/r/7048/diff/2/?file=152776#file152776line301> > > > > memory leak? great catch, this is actually a memory leak when the future is failed or discarded! Fixed here and in process.cpp as well where we have another leak, it's definitely very tricky for me to understand whether the shared_ptr and shared_array usage is correct with binds.. please take a hard look, and I'll have benh look as well - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7048/#review11443 ----------------------------------------------------------- On Sept. 12, 2012, 12:23 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7048/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2012, 12:23 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Description > ------- > > see above > > also: > -removed redundant length field from json > -added missing test for buffered io::read > > > Diffs > ----- > > src/files/files.cpp 806aa35 > src/tests/files_tests.cpp 6ef2004 > src/webui/master/static/jquery.pailer.js edd23d9 > third_party/libprocess/src/tests.cpp 41bf973 > > Diff: https://reviews.apache.org/r/7048/diff/ > > > Testing > ------- > > make check on osx, and redhat > > mesos-local.sh to verify pailer > > > Thanks, > > Ben Mahler > >
