From reviews-return-19851-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Jan 11 20:26:32 2016 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 416A818372 for ; Mon, 11 Jan 2016 20:26:32 +0000 (UTC) Received: (qmail 92193 invoked by uid 500); 11 Jan 2016 20:26:32 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 92164 invoked by uid 500); 11 Jan 2016 20:26:32 -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 92147 invoked by uid 99); 11 Jan 2016 20:26:31 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 11 Jan 2016 20:26:31 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 2F23E280614; Mon, 11 Jan 2016 20:26:31 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============3887617256517957137==" MIME-Version: 1.0 Subject: Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`. From: "Alex Clemmer" To: "Artem Harutyunyan" , "Joris Van Remoortere" , "Michael Hopcroft" , "Joseph Wu" Cc: "Alex Clemmer" , "mesos" , "Benjamin Bannier" Date: Mon, 11 Jan 2016 20:26:31 -0000 Message-ID: <20160111202631.26386.6381@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Alex Clemmer" X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/39803/ X-Sender: "Alex Clemmer" References: <20151104010213.28714.15330@reviews.apache.org> In-Reply-To: <20151104010213.28714.15330@reviews.apache.org> Reply-To: "Alex Clemmer" X-ReviewRequest-Repository: mesos --===============3887617256517957137== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line 40 > > > > > > Might add a comment explaining that when ::stat() returns a value less than zero the error can only be ENOENT or EINVAL. ENOENT means the path doesn't exist, so returning false is correct. EINVAL should be impossible, based on inspection of the code. > > Alex Clemmer wrote: > Just so we're all on the same page, let me state my understanding. Unless I'm reading the POSIX spec[1] and the Windows documentation[2], there isn't an error case where we'd want to return true, right? So it's not clear to me that adding a comment actually helps understanding. The argument I can see in favor of adding the comment is that the Windows implementation returns only two types of error codes, which is definitely less meaningful than the POSIX spec, but on balance, if the POSIX spec returns many more error codes (minus the weird Windows use of `EINVAL`), and it's still clear to readers, than maybe that means we don't need the comment. > > What do you think? > > > [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/stat.html > [2] https://msdn.microsoft.com/en-us/library/14h5k7ff.aspx Marking this as dropped; please open it up again if you see a clear reason we would want to have a comment here. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review105007 ----------------------------------------------------------- On Jan. 5, 2016, 12:12 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39803/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2016, 12:12 a.m.) > > > Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van Remoortere, and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Windows: Implemented stout/os/stat.hpp`. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 5b38b9af654d7d1c574f0cc573083b66693ced1d > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp d46e262e0fd1c2de36f3bf19d8bd693c23bf58cd > > Diff: https://reviews.apache.org/r/39803/diff/ > > > Testing > ------- > > `make check` from autotools on Ubuntu 15. > `make check` from CMake on OS X 10.10. > Ran `check` project in VS on Windows 10. > > > Thanks, > > Alex Clemmer > > --===============3887617256517957137==--