> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/68021/diff/15/?file=2120370#file2120370line49>
> >
> >     equivalent to `if(ret == -1 || EFAULT == errno)`?

https://www.wolframalpha.com/input/?i=!(a+%26+b)

No, it's equivalent to `(ret != -1 || EFAULT != errno)`.

I think Chromium devs disagree with your proposal:
https://chromium.googlesource.com/chromium/src/sandbox/+/refs/heads/master/linux/seccomp-bpf/sandbox_bpf.cc#44


> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 62-63 (patched)
> > <https://reviews.apache.org/r/68021/diff/15/?file=2120370#file2120370line62>
> >
> >     I would cache it and just parse the default profile once. Let's 
> > introduce a private variable:
> >     
> >     Option<ContainerSeccompProfile> defaultProfile;

Parsing step is very quick. Caching it just makes the code more complicated. 
Since parsing a profile is not an issue, while caching it adds more complexity, 
I don't think we need to do that.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9035
>     https://issues.apache.org/jira/browse/MESOS-9035
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf 
>   src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 5016f2e9f0651abcb0a5f364e8eace458f2edeae 
>   src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68021/diff/15/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Reply via email to