> 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 > >