----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68644/#review209489 -----------------------------------------------------------
3rdparty/libprocess/src/posix/subprocess.hpp Lines 213 (patched) <https://reviews.apache.org/r/68644/#comment293959> Since you are planning a different code path for macOS, maybe hoist this out into a static support function in preparation? 3rdparty/libprocess/src/posix/subprocess.hpp Lines 214 (patched) <https://reviews.apache.org/r/68644/#comment293955> How about we rephrase this as: ``` Close all file descriptors that are not explicitly whitelisted to avoid ... ``` 3rdparty/libprocess/src/posix/subprocess.hpp Lines 233 (patched) <https://reviews.apache.org/r/68644/#comment293958> We should explicitly: ```C #include <sys/syscall.h> ``` 3rdparty/libprocess/src/posix/subprocess.hpp Lines 244 (patched) <https://reviews.apache.org/r/68644/#comment293957> I think our guidelines would say this should use `reinterpret_cast`? 3rdparty/libprocess/src/posix/subprocess.hpp Lines 255 (patched) <https://reviews.apache.org/r/68644/#comment293956> You can just use `std::find()` here. 3rdparty/libprocess/src/posix/subprocess.hpp Lines 275 (patched) <https://reviews.apache.org/r/68644/#comment293954> 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()`? - James Peach On Oct. 11, 2018, 2: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, 2: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 > >
