> 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? > > Alex Clemmer wrote: > 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?
Per our discussion on Slack, and our consulting of the documentation[1][2], and also inspection of the source code, we have concluded it is safe to call `*Close` on an invalid handle after all. Hence, let's close the comment as "drop". [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364413(v=vs.85).aspx [2] https://msdn.microsoft.com/en-us/library/windows/desktop/ms724211(v=vs.85).aspx - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39584/#review110385 ----------------------------------------------------------- On Jan. 7, 2016, 8:29 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39584/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2016, 8:29 p.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 > >
