> On Sept. 20, 2012, 2 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/stout/os.hpp, line 192 > > <https://reviews.apache.org/r/7184/diff/2/?file=158674#file158674line192> > > > > This should really use path::join (here and elsewhere).
done > On Sept. 20, 2012, 2 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/stout/os.hpp, line 323 > > <https://reviews.apache.org/r/7184/diff/2/?file=158674#file158674line323> > > > > Let's not kill the program here please. This will need to return a Try > > after all. > > Benjamin Hindman wrote: > To elaborate, consider what might happen if os::basename or os::dirname > got called on an argument passed to the master or slave via an HTTP request. > In that case, we SHOULD NOT be crashing the master/slave because someone > passed the string > "some_string_which_if_constructed_properly_would_cause_basename_to_fail_thus_killing_the_master_or_slave". > Instead, the HTTP request should return a BadRequest. I understand that this > is a bit of an involved change, but it needs to be done (and your review is > just illuminating this case). > > Vinod Kone wrote: > agreed. i started going the Try route, for exactly the same reasons > listed above, but punted on it on account of laziness :) will fix it. done. NOTE: i've used .get() directly, wherever I deemed a crash is acceptable. for everything else, i did the check, return if error, dance. > On Sept. 20, 2012, 2 a.m., Benjamin Hindman wrote: > > third_party/libprocess/include/stout/os.hpp, line 337 > > <https://reviews.apache.org/r/7184/diff/2/?file=158674#file158674line337> > > > > Ditto. done - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7184/#review11727 ----------------------------------------------------------- On Sept. 20, 2012, 1:45 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7184/ > ----------------------------------------------------------- > > (Updated Sept. 20, 2012, 1:45 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Also fixes mktemp and mkdtemp > > > Diffs > ----- > > src/Makefile.am df86969 > src/common/type_utils.hpp f80cfce > src/examples/balloon_executor.cpp 3c84436 > src/slave/paths.hpp 0c817ca > src/tests/slave_state_tests.cpp d18e8a4 > third_party/libprocess/include/process/async.hpp d71b9ee > third_party/libprocess/include/stout/os.hpp 275e5ae > > Diff: https://reviews.apache.org/r/7184/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
