> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 130 > > <https://reviews.apache.org/r/39803/diff/2/?file=1113684#file1113684line130> > > > > Might want to static_assert that _USE_32BIT_TIME_T is not defined. > > Probably sound advice for _UNICODE and _MBCS as well. Perhaps these static > > asserts should go in windows.hpp? If you put them there, add a comment > > saying why the asserts are necessary (e.g. "Implementation of mtime() > > assumes ...") > > Alex Clemmer wrote: > Thinking about it now, I think we should assert this at the top of > `stat.hpp`, because we're not guaranteed to include `windows.hpp` at all in > general, and besides that, we're not guaranteed to include it after defining > `_USER_32BIT_TIME_T`. > > `stat.hpp` file is meant to wrap calls to `stat` anyway, so I think this > is probably sufficient. > > What do you think? > > Alex Clemmer wrote: > As for `_MBCS` and `_UNICODE`, I think we can put such things in > `windows.hpp`, but I think there is probably a better place for them, > considering that this really only will be effective at protecting against -D > flags passed into the compiler.
Alright, I've gone ahead and added checks for `_USER_32BIT_TIME_T` and `_UNICODE`, but `_MBCS` seems to be enabled in VS projects by default. Consulting with Alex Naparu, we believe it is not important to explicitly check that this flag is not set. We do agree that `_UNICODE` should not be set because it makes POSIX compliance really hard. Let us know if our understanding is not correct here. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review105083 ----------------------------------------------------------- On Jan. 12, 2016, 2:03 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39803/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2016, 2:03 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/posix/stat.hpp > ffe074830ef90f492990bf695e686c885b9a155c > 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 > >
