mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 60495: Added network ports isolator listen socket utilities.
Date Tue, 12 Sep 2017 08:27:59 GMT


> On Sept. 8, 2017, 3:43 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Line 112 (original), 112 (patched)
> > <https://reviews.apache.org/r/60495/diff/13-15/?file=1808218#file1808218line112>
> >
> >     Why do we need a `for` loop like this? I think the previous `while` loop is
good enough.
> 
> James Peach wrote:
>     If we want to depend on `errno`, we need to guarantee that it is `0` before we call
the `readddir`. Since there is non-trivial logic inside the loop, it is difficult to be confident
that nothing we call is going to change it, particularly if this code is ever updated.

OK, then what about a `while` loop and setting `errno` to 0 in the end of each iteration?
Setting `errno` to 0 in the `for` loop seems a bit strange to me.


- Qian


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


On Sept. 8, 2017, 5:50 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60495/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2017, 5:50 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
>     https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented network ports isolator socket utilities to find the
> inode numbers of all the listening sockets, and the inodes of the
> open sockets for a process. Together, these utilities can tell us
> which sockets a process is listening on.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60495/diff/15/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>


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