> On Sept. 1, 2016, 2:05 a.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/posix/subprocess.hpp, lines 55-68 > > <https://reviews.apache.org/r/51378/diff/6/?file=1486771#file1486771line55> > > > > Any reason you need to expose this internal method? Why not just write > > your own clone function at the call sites? THis default clone sounds pretty > > straight forward to me. Copy that to the call site is probably more > > readable. > > > > `process::defaultClone` just sounds like a very weird public interface. > > > > I'd suggest we revert this change.
I'd rather not duplicate code. Copying is not a problem, but since currently—read: without child hooks—the only way to add something between `fork` and `exec` is to write a custom `clone`, I don't want people to deviate and write all kind of `clone`s: based on `fork`, based on `clone`, with different stack options... This potentially will lead to hardly debuggable issues. I'd suggest we add (probably Windows-aware) child hooks and _then_ mark this function private again. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51378/#review147525 ----------------------------------------------------------- On Aug. 28, 2016, 1:55 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51378/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2016, 1:55 p.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón > Kleiman, Gilbert Song, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-5961 > https://issues.apache.org/jira/browse/MESOS-5961 > > > Repository: mesos > > > Description > ------- > > Exposed `process::internal::defaultClone` to `process` namespace. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/posix/subprocess.hpp > a871fe484905165eed093124687c4920f3968ccc > > Diff: https://reviews.apache.org/r/51378/diff/ > > > Testing > ------- > > See https://reviews.apache.org/r/51379/ > > > Thanks, > > haosdent huang > >
