----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60495/#review184953 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/ports.cpp Line 112 (original), 112 (patched) <https://reviews.apache.org/r/60495/#comment261177> Why do we need a `for` loop like this? I think the previous `while` loop is good enough. src/slave/containerizer/mesos/isolators/network/ports.cpp Line 124 (original), 125 (patched) <https://reviews.apache.org/r/60495/#comment261178> This seems too strict, since here we are already going to return an error out, I think we can simply ignore the error of `closedir()`. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 140 (patched) <https://reviews.apache.org/r/60495/#comment261179> Ditto. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 144 (patched) <https://reviews.apache.org/r/60495/#comment261180> Better to change this to: ``` if (closedir(dir) == -1) { return ErrnoError("Failed to close directory '" + fdPath + "'"); } ``` This is also aligned with how you handle the error of `opendir()` in the above. - Qian Zhang 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 > >
