mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Clemmer" <clemmer.alexan...@gmail.com>
Subject Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.
Date Mon, 11 Jan 2016 20:26:31 GMT


> On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line 40
> > <https://reviews.apache.org/r/39803/diff/2/?file=1113684#file1113684line40>
> >
> >     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
> 
>


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