Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-13 Thread Ben Mahler 

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

Ship it!


Thanks!


src/files/files.cpp (lines 350 - 351)


How about:

```
string error = strings::format(
"Failed to open file at '%s': %s",
resolvedPath.get(),
os::strerror(errno)).get();
```

Ditto below.



src/launcher/executor.cpp (lines 48 - 49)


Whoops, double include?



src/linux/routing/link/link.cpp (line 49)


Per my previous comments, we don't need to introduce os/strerror.hpp 
includes where we already include os.hpp since it's all encompassing.



src/tests/script.cpp (lines 164 - 165)


Why the wrapping change here?


- Ben Mahler


On Nov. 12, 2015, 3:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Nov. 12, 2015, 3:27 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp f0d8a21739a52748c2e8d819b2713ae626c1444d 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 435631d3c4e436906bc7d20830c32d1910fb54fe 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 38fd8684d11c7a3e0499540bf6458258d58cd2b2 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> b0a15d4e9a45c0fb5e3dffe181730df8149cc44f 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> cc54e905d6c82128a578917de3c91e69d953b23b 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 470221121d60d3683c3200bd95d334469deb560c 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 22ca3c197a90d1f19989b1ed826a22237559583b 
>   src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp d2280c2430db1e051de4d07bf8b04713723fbeb3 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-12 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39005, 39008]

All tests passed.

- Mesos ReviewBot


On Nov. 12, 2015, 3:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Nov. 12, 2015, 3:27 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp f0d8a21739a52748c2e8d819b2713ae626c1444d 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 435631d3c4e436906bc7d20830c32d1910fb54fe 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 38fd8684d11c7a3e0499540bf6458258d58cd2b2 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> b0a15d4e9a45c0fb5e3dffe181730df8149cc44f 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> cc54e905d6c82128a578917de3c91e69d953b23b 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 470221121d60d3683c3200bd95d334469deb560c 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 22ca3c197a90d1f19989b1ed826a22237559583b 
>   src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp d2280c2430db1e051de4d07bf8b04713723fbeb3 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-12 Thread Benjamin Bannier 

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

(Updated Nov. 12, 2015, 3:27 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Fixed deps.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp f0d8a21739a52748c2e8d819b2713ae626c1444d 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 435631d3c4e436906bc7d20830c32d1910fb54fe 
  src/slave/containerizer/mesos/containerizer.cpp 
38fd8684d11c7a3e0499540bf6458258d58cd2b2 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
b0a15d4e9a45c0fb5e3dffe181730df8149cc44f 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
cc54e905d6c82128a578917de3c91e69d953b23b 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
470221121d60d3683c3200bd95d334469deb560c 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
22ca3c197a90d1f19989b1ed826a22237559583b 
  src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp d2280c2430db1e051de4d07bf8b04713723fbeb3 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-12 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [39005, 39006, 39007]

Failed command: ./support/apply-review.sh -n -r 39007

Error:
 2015-11-12 15:16:29 URL:https://reviews.apache.org/r/39007/diff/raw/ 
[7614/7614] -> "39007.patch" [1]
error: patch failed: 3rdparty/libprocess/src/subprocess.cpp:176
error: 3rdparty/libprocess/src/subprocess.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Nov. 12, 2015, 3:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Nov. 12, 2015, 3:13 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp f0d8a21739a52748c2e8d819b2713ae626c1444d 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 435631d3c4e436906bc7d20830c32d1910fb54fe 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 38fd8684d11c7a3e0499540bf6458258d58cd2b2 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> b0a15d4e9a45c0fb5e3dffe181730df8149cc44f 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> cc54e905d6c82128a578917de3c91e69d953b23b 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 470221121d60d3683c3200bd95d334469deb560c 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 22ca3c197a90d1f19989b1ed826a22237559583b 
>   src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp d2280c2430db1e051de4d07bf8b04713723fbeb3 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-12 Thread Benjamin Bannier 

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

(Updated Nov. 12, 2015, 3:13 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp f0d8a21739a52748c2e8d819b2713ae626c1444d 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 435631d3c4e436906bc7d20830c32d1910fb54fe 
  src/slave/containerizer/mesos/containerizer.cpp 
38fd8684d11c7a3e0499540bf6458258d58cd2b2 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
b0a15d4e9a45c0fb5e3dffe181730df8149cc44f 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
cc54e905d6c82128a578917de3c91e69d953b23b 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
470221121d60d3683c3200bd95d334469deb560c 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
22ca3c197a90d1f19989b1ed826a22237559583b 
  src/slave/slave.cpp ac2d0e0153721a66495cd6539b25f5b3cee9d979 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp d2280c2430db1e051de4d07bf8b04713723fbeb3 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-11-12 Thread Bernd Mathiske 

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

Ship it!


Assuming that we will follow through with MESOS-3607.

- Bernd Mathiske


On Oct. 22, 2015, 12:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 22, 2015, 12:52 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-22 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39005, 39008]

All tests passed.

- Mesos ReviewBot


On Oct. 22, 2015, 7:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 22, 2015, 7:52 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-22 Thread Benjamin Bannier 


> On Oct. 21, 2015, 10:22 p.m., Kapil Arya wrote:
> > src/launcher/executor.cpp, line 49
> > 
> >
> > This file should go at the end of the stout includes ... don't we put 
> > stout/XX.hpp before stout/DIR/XX.hpp?

Let's keep lexicographical sort with an ASCII code page (which plays nicely 
with everything from sort to clang-format) while the discussion for the coding 
guidelines MESOS-2275 is still in flux.


- Benjamin


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


On Oct. 22, 2015, 7:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 22, 2015, 7:52 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-22 Thread Benjamin Bannier 

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

(Updated Oct. 22, 2015, 7:52 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Corrected dependency chain.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-22 Thread Benjamin Bannier 

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

(Updated Oct. 22, 2015, 7:51 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Rebased and incorporated review comments.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-21 Thread Kapil Arya 

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

Ship it!


LGTM modulo some comments about #includes.


src/cli/mesos.cpp (line 26)


#include order, please see the comments below.



src/files/files.cpp (line 47)


same here.



src/launcher/executor.cpp (line 49)


This file should go at the end of the stout includes ... don't we put 
stout/XX.hpp before stout/DIR/XX.hpp?



src/linux/routing/link/internal.hpp (line 150)


`#include ` here too?



src/linux/routing/link/link.cpp (line 300)


`#include ` here too?



src/slave/containerizer/isolators/filesystem/posix.cpp (line 24)


ditto.



src/slave/containerizer/mesos/containerizer.cpp (line 33)


ditto.



src/slave/containerizer/mesos/launch.cpp (line 27)


ditto.



src/slave/slave.cpp (line 56)


Same as above regarding the #include order.



src/tests/containerizer/memory_test_helper.cpp (line 36)


ditto



src/tests/script.cpp (line 28)


ditto


- Kapil Arya


On Oct. 12, 2015, 3:09 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 12, 2015, 3:09 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 174448cc039b316329a8617f488f037a1e640e23 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-12 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39005, 39008]

All tests passed.

- Mesos ReviewBot


On Oct. 12, 2015, 7:09 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 12, 2015, 7:09 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 174448cc039b316329a8617f488f037a1e640e23 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-12 Thread Benjamin Bannier 

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

(Updated Oct. 12, 2015, 7:09 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Fixed includes for Linux.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 174448cc039b316329a8617f488f037a1e640e23 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-09 Thread Benjamin Bannier 

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

(Updated Oct. 9, 2015, 2:25 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Added another missing header.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-08 Thread Benjamin Bannier 

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

(Updated Oct. 8, 2015, 6:15 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Updated includes.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-08 Thread Benjamin Bannier 

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

(Updated Oct. 8, 2015, 2:39 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Adjusted for rename `stringError` -> `os::strerror` in RR-39005.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-08 Thread Benjamin Bannier 

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

(Updated Oct. 8, 2015, 2:31 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Changes
---

Adjusted for rename `stringError` -> `os::strerror` in RR-39007.


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


Repository: mesos


Description
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-08 Thread Benjamin Bannier 


> On Oct. 7, 2015, 9:18 p.m., Ben Mahler wrote:
> > src/tests/script.cpp, line 101
> > 
> >
> > malloc is not async-signal-safe, and may be used by the string 
> > implementation, and so we must not use os::strerror (assuming we rename it) 
> > after a fork! Please do an audit of this diff.
> 
> Bernd Mathiske wrote:
> Yikes. Good catch. I glossed over the fork. My bad.

Good catch, I have filed MESOS-3607. I suggest this to be fixed separately 
since more than a few of the functions in that fork/exec bracket are not 
async-safe right now (strerror was not safe either).


- Benjamin


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


On Oct. 6, 2015, 3:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 3:08 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-08 Thread Bernd Mathiske 


> On Oct. 7, 2015, 2:18 p.m., Ben Mahler wrote:
> > src/tests/script.cpp, line 101
> > 
> >
> > malloc is not async-signal-safe, and may be used by the string 
> > implementation, and so we must not use os::strerror (assuming we rename it) 
> > after a fork! Please do an audit of this diff.

Yikes. Good catch. I glossed over the fork. My bad.


- Bernd


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


On Oct. 6, 2015, 8:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 8:08 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-07 Thread Ben Mahler 

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



src/tests/script.cpp (line 100)


malloc is not async-signal-safe, and may be used by the string 
implementation, and so we must not use os::strerror (assuming we rename it) 
after a fork! Please do an audit of this diff.


- Ben Mahler


On Oct. 6, 2015, 3:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 3:08 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39008: Used thread-safe replacement for strerror.

2015-10-07 Thread Bernd Mathiske 

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

Ship it!


Once it compiles (when the preceding patch is fixed), LGTM.

- Bernd Mathiske


On Oct. 6, 2015, 8:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39008/
> ---
> 
> (Updated Oct. 6, 2015, 8:08 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch call sites to using safe strerror_r wrapper.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
>   src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/linux/routing/link/internal.hpp 
> 015c0ef5be516d7786c96a96437cced1ae8487fa 
>   src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
>   src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/containerizer/mesos/launch.cpp 
> 09d4d8f4d6837e93a82deef76ca07e2167d6a405 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
>   src/tests/containerizer/memory_test_helper.cpp 
> 8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
>   src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 
> 
> Diff: https://reviews.apache.org/r/39008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 39008: Used thread-safe replacement for strerror.

2015-10-06 Thread Benjamin Bannier 

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier