From reviews-return-65873-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Tue Sep 12 08:28:06 2017 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E72381A5B4 for ; Tue, 12 Sep 2017 08:28:05 +0000 (UTC) Received: (qmail 63405 invoked by uid 500); 12 Sep 2017 08:28:05 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 63373 invoked by uid 500); 12 Sep 2017 08:28:05 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 63362 invoked by uid 99); 12 Sep 2017 08:28:05 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 12 Sep 2017 08:28:05 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 020C61A6DDB; Tue, 12 Sep 2017 08:28:05 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 4.451 X-Spam-Level: **** X-Spam-Status: No, score=4.451 tagged_above=-999 required=6.31 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_REPLYTO_END_DIGIT=0.25, HEADER_FROM_DIFFERENT_DOMAINS=0.001, HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, NML_ADSP_CUSTOM_MED=1.2, RP_MATCHES_RCVD=-0.001] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id iCwwiTOhj1i7; Tue, 12 Sep 2017 08:28:03 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTP id 26A6C5F640; Tue, 12 Sep 2017 08:28:03 +0000 (UTC) Received: from reviews.apache.org (unknown [10.41.0.12]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 7B8E1E010F; Tue, 12 Sep 2017 08:28:02 +0000 (UTC) Received: from reviews-vm2.apache.org (localhost [IPv6:::1]) by reviews.apache.org (ASF Mail Server at reviews-vm2.apache.org) with ESMTP id B490DC400A2; Tue, 12 Sep 2017 08:27:59 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4528882565516684871==" MIME-Version: 1.0 Subject: Re: Review Request 60495: Added network ports isolator listen socket utilities. From: Qian Zhang To: Qian Zhang , Jiang Yan Xu Cc: James Peach , mesos Date: Tue, 12 Sep 2017 08:27:59 -0000 Message-ID: <20170912082759.46133.51617@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Qian Zhang X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/60495/ X-Sender: Qian Zhang References: <20170908074306.9617.51624@reviews-vm2.apache.org> In-Reply-To: <20170908074306.9617.51624@reviews-vm2.apache.org> X-ReviewBoard-Diff-For: src/slave/containerizer/mesos/isolators/network/ports.hpp X-ReviewBoard-Diff-For: src/slave/containerizer/mesos/isolators/network/ports.cpp Reply-To: Qian Zhang X-ReviewRequest-Repository: mesos --===============4528882565516684871== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Sept. 8, 2017, 3:43 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/ports.cpp > > Line 112 (original), 112 (patched) > > > > > > 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 > > --===============4528882565516684871==--