mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 68644: Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Date Fri, 12 Oct 2018 17:40:58 GMT

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


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