Re: Review Request 38699: stout: use os::close everywhere.

2015-09-23 Thread Cong Wang 

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp (line 266)


I must miss something too obvious, but... doesn't os::close() return a 
Try? How could that be compared with -1?


- Cong Wang


On Sept. 24, 2015, 12:21 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38699/
> ---
> 
> (Updated Sept. 24, 2015, 12:21 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: mesos-2768
> https://issues.apache.org/jira/browse/mesos-2768
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> stout: use os::close everywhere.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> 7eb51e8771e95f57548fc35753e75c6d56cd97cd 
> 
> Diff: https://reviews.apache.org/r/38699/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Review Request 38699: stout: use os::close everywhere.

2015-09-23 Thread Chi Zhang 

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

Review request for mesos and Vinod Kone.


Bugs: mesos-2768
https://issues.apache.org/jira/browse/mesos-2768


Repository: mesos


Description
---

stout: use os::close everywhere.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
7eb51e8771e95f57548fc35753e75c6d56cd97cd 

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


Testing
---


Thanks,

Chi Zhang

Re: Review Request 38699: stout: use os::close everywhere.

2015-09-23 Thread Ben Mahler 

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp (line 266)


Is it possible for the shared memory deleter to execute in the forked 
context? Or are we guaranteed that the last reference is in the parent? If not, 
this change seems problematic since close is not async signal safe (Try uses 
new which uses malloc). It looks like ABORT with std::string is also 
problematic in the error case.


- Ben Mahler


On Sept. 24, 2015, 12:21 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38699/
> ---
> 
> (Updated Sept. 24, 2015, 12:21 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: mesos-2768
> https://issues.apache.org/jira/browse/mesos-2768
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> stout: use os::close everywhere.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> 7eb51e8771e95f57548fc35753e75c6d56cd97cd 
> 
> Diff: https://reviews.apache.org/r/38699/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>