Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-08-25 Thread Michael Park 

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1423)


```
add(&hostname, "hostname", "Hostname of the container");
```


- Michael Park


On Aug. 25, 2016, 5:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46822/
> ---
> 
> (Updated Aug. 25, 2016, 5:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While `FlagsBase` internally stores just maps of names and
> flags, the functions stored in a `Flag` rely on the original type of
> the `Flags` containing them (e.g., we perform dynamic casts to detect
> their types).
> 
> Since `Option` stores `T` as a value (i.e., it cannot contain
> reference types) any interface taking an `Option` cannot rely on
> polymorphic behavior of `T`. To make use of polymorphism we should
> instead store e.g., a pointer type to avoid slicing.
> 
> Here we change `Flags` arguments of `subprocess` to allow preserving
> the original type so `Flag` functions can work reliably; we do this by
> passing a non-owning pointer to a `Flags` so we do not restrict copying.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
>   src/health-check/health_checker.cpp 
> d43cb0568c120cbcec6a73d232396ccc54cf3e58 
>   src/launcher/posix/executor.cpp 43573cacee4e681d4327a7ed7c43b4ee263aa175 
>   src/linux/perf.cpp 9455210064779b59ad56637d846fe7584b21340f 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 1fca4864457388b265779fbca72296336f1aa5a9 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> 842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 760d32bf3dc09f3b715b378f5ded41556f15fe41 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 92f3c07e285ad3b8ef26692aa6475d755188b469 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> f97ace9579879b35cb8050c56e968a31bb741c55 
>   src/slave/containerizer/mesos/launcher.hpp 
> bf435e3a9c150648336a1becf2f075fa183428bd 
>   src/slave/containerizer/mesos/launcher.cpp 
> 9efe8474a2210957ce256fc08cb35694194213c3 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> c1852226c74bc611d045be721e284141e59adcd9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 95dee95c5e6e613e526c92d8729ae5583c8b58f1 
>   src/tests/containerizer/isolator_tests.cpp 
> 4f047ae6b2e85e177e8b73d60b9dfca913c832a5 
>   src/tests/containerizer/launch_tests.cpp 
> ef0c87c96e6a8379d119246a8ad044248522e67e 
>   src/tests/containerizer/launcher.hpp 
> 7e5c243efad11d04e70b36876b2ed4db82666d31 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e3c8daab4fc688150a4f222e05f9b1bd9aee1912 
>   src/tests/containerizer/ns_tests.cpp 
> cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
>   src/tests/containerizer/port_mapping_tests.cpp 
> fd181cae5540de1fdd631367aba5cce249f1b72c 
> 
> Diff: https://reviews.apache.org/r/46822/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-08-11 Thread Benjamin Bannier 

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

(Updated Aug. 11, 2016, 8:24 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Rebased.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/health-check/health_checker.cpp d43cb0568c120cbcec6a73d232396ccc54cf3e58 
  src/launcher/posix/executor.cpp 43573cacee4e681d4327a7ed7c43b4ee263aa175 
  src/linux/perf.cpp 9455210064779b59ad56637d846fe7584b21340f 
  src/slave/container_loggers/lib_logrotate.cpp 
1fca4864457388b265779fbca72296336f1aa5a9 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
760d32bf3dc09f3b715b378f5ded41556f15fe41 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
92f3c07e285ad3b8ef26692aa6475d755188b469 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
f97ace9579879b35cb8050c56e968a31bb741c55 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
4f047ae6b2e85e177e8b73d60b9dfca913c832a5 
  src/tests/containerizer/launch_tests.cpp 
ef0c87c96e6a8379d119246a8ad044248522e67e 
  src/tests/containerizer/launcher.hpp 7e5c243efad11d04e70b36876b2ed4db82666d31 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
e3c8daab4fc688150a4f222e05f9b1bd9aee1912 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/port_mapping_tests.cpp 
fd181cae5540de1fdd631367aba5cce249f1b72c 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-07-05 Thread Benjamin Bannier 


> On May 11, 2016, 10:46 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 1272
> > 
> >
> > why do you need to do this namespacing? doesn't seem related to the 
> > rest of the patch?

This slipped in here. I moved this to a patch later in the series.


- Benjamin


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


On July 5, 2016, 7:46 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46822/
> ---
> 
> (Updated July 5, 2016, 7:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
> Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While `FlagsBase` internally stores just maps of names and
> flags, the functions stored in a `Flag` rely on the original type of
> the `Flags` containing them (e.g., we perform dynamic casts to detect
> their types).
> 
> Since `Option` stores `T` as a value (i.e., it cannot contain
> reference types) any interface taking an `Option` cannot rely on
> polymorphic behavior of `T`. To make use of polymorphism we should
> instead store e.g., a pointer type to avoid slicing.
> 
> Here we change `Flags` arguments of `subprocess` to allow preserving
> the original type so `Flag` functions can work reliably; we do this by
> passing a non-owning pointer to a `Flags` so we do not restrict copying.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
>   src/health-check/health_checker.hpp 
> f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
>   src/launcher/posix/executor.cpp 6814b9f2333a9f040228c41e1d7101b93c760c01 
>   src/linux/perf.cpp ea823b32edaa82a71cbfac9932aae543c0d7bef4 
>   src/slave/container_loggers/lib_logrotate.cpp 
> d53847bbca83367927725f06e8f962ce7011f468 
>   src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> 842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 92b33111799cb4e1c8bc2051381e1254d701d95c 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
>   src/slave/containerizer/mesos/launcher.hpp 
> bf435e3a9c150648336a1becf2f075fa183428bd 
>   src/slave/containerizer/mesos/launcher.cpp 
> 9efe8474a2210957ce256fc08cb35694194213c3 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> c1852226c74bc611d045be721e284141e59adcd9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 95dee95c5e6e613e526c92d8729ae5583c8b58f1 
>   src/tests/containerizer/isolator_tests.cpp 
> 488747347f71a6a1bb6bc01477143d077d4fd3eb 
>   src/tests/containerizer/launch_tests.cpp 
> ef0c87c96e6a8379d119246a8ad044248522e67e 
>   src/tests/containerizer/launcher.hpp 
> 49f8ecb47abb098bf76017f79b7d00380401efe4 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 57588cc1fb918924a163bdb40b195cc5f4231f6e 
>   src/tests/containerizer/ns_tests.cpp 
> cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 3675c02453160417b24c8b3b0d001208504515a5 
> 
> Diff: https://reviews.apache.org/r/46822/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-07-04 Thread Benjamin Bannier 

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

(Updated July 5, 2016, 7:46 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/launcher/posix/executor.cpp 6814b9f2333a9f040228c41e1d7101b93c760c01 
  src/linux/perf.cpp ea823b32edaa82a71cbfac9932aae543c0d7bef4 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 
  src/tests/containerizer/launch_tests.cpp 
ef0c87c96e6a8379d119246a8ad044248522e67e 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
57588cc1fb918924a163bdb40b195cc5f4231f6e 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/port_mapping_tests.cpp 
3675c02453160417b24c8b3b0d001208504515a5 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-07-04 Thread Benjamin Bannier 

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

(Updated July 5, 2016, 1:13 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Fixed misses Linux flags uses.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/linux/perf.cpp ea823b32edaa82a71cbfac9932aae543c0d7bef4 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 
  src/tests/containerizer/launch_tests.cpp 
ef0c87c96e6a8379d119246a8ad044248522e67e 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
57588cc1fb918924a163bdb40b195cc5f4231f6e 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/port_mapping_tests.cpp 
3675c02453160417b24c8b3b0d001208504515a5 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-07-04 Thread Benjamin Bannier 

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

(Updated July 4, 2016, 2:41 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Pass `Flags` by raw ptr.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description (updated)
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
a523799b3bd8a69c3dcd5a47ed9d81eace906686 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
7b4d47bd9e99b71269093d7c11559f3b74a3e22b 
  src/tests/containerizer/launch_tests.cpp 
3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
7ffec9ec9788d0e15c2c0ccc7d10b8b64ad40057 
  src/tests/containerizer/port_mapping_tests.cpp 
1c50cf545392488cf1e2d51d8e03887bebef5e75 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-07-03 Thread Benjamin Bannier 

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

(Updated July 3, 2016, 5:22 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Rebased.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` function can work reliably. We do this by
passing a `Shared` so we do not restrict copying; not that it
would also be possible to use an `Owned`, but this would
require an audit of all sites where the arguments are used as
`Owned` should not be copied, but do not prevent that on their own.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp 915030bdbe5a5b55acc38042ee0475074a602ee0 
  src/slave/containerizer/mesos/containerizer.cpp 
a96b382f22886362a1159e1166dfe041072985ba 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/launcher.hpp 
05320f462653c31fc2f093d6c67e2182e9c794fa 
  src/slave/containerizer/mesos/launcher.cpp 
ff675262af8947b89f8099828665e5e5d86491d8 
  src/slave/containerizer/mesos/linux_launcher.hpp 
89bb2958a41dffe4ade9c2492b9a7412f90a432d 
  src/slave/containerizer/mesos/linux_launcher.cpp 
5028854fa003615f158120e030866b7ec4402b66 
  src/tests/containerizer/launch_tests.cpp 
3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
  src/tests/containerizer/launcher.hpp c352634c4766d289706c7cc738677619d7d02ccd 
  src/tests/containerizer/port_mapping_tests.cpp 
1c50cf545392488cf1e2d51d8e03887bebef5e75 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-05-11 Thread Vinod Kone 

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



the summary and description in the review are incorrect. can you please update?


src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1272)


why do you need to do this namespacing? doesn't seem related to the rest of 
the patch?


- Vinod Kone


On May 11, 2016, 12:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46822/
> ---
> 
> (Updated May 11, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
> Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While `FlagsBase` internally stores just maps of names and
> flags, the functions stored in a `Flag` rely on the original type of
> the `Flags` containing them (e.g., we perform dynamic casts to detect
> their types).
> 
> Since `Option` stores `T` as a value (i.e., it cannot contain
> reference types) any interface taking an `Option` cannot rely on
> polymorphic behavior of `T`. To make use of polymorphism we should
> instead store e.g., a pointer type to avoid slicing.
> 
> Here we change `Flags` arguments of `subprocess` to allow preserving
> the original type so `Flag` function can work reliably. We do this by
> passing a `Shared` so we do not restrict copying; not that it
> would also be possible to use an `Owned`, but this would
> require an audit of all sites where the arguments are used as
> `Owned` should not be copied, but do not prevent that on their own.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 1f228806da32832c9ca1ae4defcd1bdc154adc18 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> dae369aadb940150aa806b28d9269e3d88cf57ed 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> ad792def2bb3a1614d21ca28d858e400d2e3ede1 
>   src/slave/containerizer/mesos/launcher.hpp 
> 5977c30c0aacc569019f7b34bb0c6577823ec887 
>   src/slave/containerizer/mesos/launcher.cpp 
> a5c8c31b72773d0bd10b9d02675a01f1d641d41c 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launch_tests.cpp 
> 3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 
> 
> Diff: https://reviews.apache.org/r/46822/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46822: Avoided slicing of flags in subprocess.

2016-05-11 Thread Benjamin Bannier 

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

(Updated May 11, 2016, 2:14 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Rebased.


Bugs: MESOS-3335
https://issues.apache.org/jira/browse/MESOS-3335


Repository: mesos


Description (updated)
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` function can work reliably. We do this by
passing a `Shared` so we do not restrict copying; not that it
would also be possible to use an `Owned`, but this would
require an audit of all sites where the arguments are used as
`Owned` should not be copied, but do not prevent that on their own.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
1f228806da32832c9ca1ae4defcd1bdc154adc18 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
  src/slave/containerizer/mesos/containerizer.cpp 
75e5a32a3e70ec60a6800e21a621673184ea0956 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
dae369aadb940150aa806b28d9269e3d88cf57ed 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
ad792def2bb3a1614d21ca28d858e400d2e3ede1 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
89bb2958a41dffe4ade9c2492b9a7412f90a432d 
  src/slave/containerizer/mesos/linux_launcher.cpp 
5028854fa003615f158120e030866b7ec4402b66 
  src/tests/containerizer/launch_tests.cpp 
3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
  src/tests/containerizer/launcher.hpp c352634c4766d289706c7cc738677619d7d02ccd 
  src/tests/containerizer/port_mapping_tests.cpp 
21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 

Diff: https://reviews.apache.org/r/46822/diff/


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier