mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 51378: Exposed `process::internal::defaultClone` to `process` namespace.
Date Thu, 01 Sep 2016 09:10:54 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message