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 39584: Windows: Implemented `os::rmdir.hpp`.
Date Mon, 04 Jan 2016 12:00:14 GMT


> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 51
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line51>
> >
> >     Calling FindClose on an invalid handle is not the best idea. At best it will
be a no-op.
> 
> Alex Clemmer wrote:
>     So, would you suggest checking if the handle is `INVALID_HANDLE_VALUE` and closing
it only if that's not true?

I bring this up mainly because it seems like if we go the RAII route, we'll end up calling
this anyway. So if it's a big deal, we need to beef up the shared_ptr dispose logic for all
our shared handles, right?


> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, lines 110-111
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line110>
> >
> >     Nit: error message is the same as the one on line 100, which makes it hard to
tell what happened, even for someone with access to the source code.

We don't really need to re-box the error on (what is now) line 81. So let's just remove that
one.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------


On Jan. 4, 2016, 11:59 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 11:59 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe

>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION

>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99

>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5

> 
> Diff: https://reviews.apache.org/r/39584/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