> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 213 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line213>
> >
> >     Since you are planning a different code path for macOS, maybe hoist 
> > this out into a static support function in preparation?

Did you mean we should put those codes into a static function like `static int 
convertStringToInt(const char *name)`?


> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 255 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line255>
> >
> >     You can just use `std::find()` here.

That's what I was thinking. But it seems `std::find()` may allocate memory 
(search `allocate memory` in https://en.cppreference.com/w/cpp/algorithm/find )?


> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 275 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line275>
> >
> >     Unfortunately, the `Try` here is not async-signal-safe. However, that 
> > is already used by `UNSET_CLOEXEC`, so I think we can just leave a TODO 
> > here.
> >     
> >     Can you file a JIRA to add something like `signal_save::uncloexec()`?

I see we also use `Try` in another place in `childMain`, e.g., `Try<Nothing> 
callback = hook();`, so that one needs to be changed too?

And I'd like to call `fcntl` here directly to unset the `close-on-exec` flag.


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209489
-----------------------------------------------------------


On Oct. 11, 2018, 10:03 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2018, 10:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152
>     https://issues.apache.org/jira/browse/MESOS-9152
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/posix/subprocess.hpp 
> 007058b61fdcd4716aa793516c842c3cef8c0a29 
>   3rdparty/libprocess/src/subprocess.cpp 
> c0640de2dc4278b884282dfaad98c49c3b067a5b 
> 
> 
> Diff: https://reviews.apache.org/r/68644/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Reply via email to