mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 62508: Fixed ordering of Windows system headers.
Date Mon, 02 Oct 2017 19:38:06 GMT


> On Oct. 2, 2017, 12:26 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/ip.hpp
> > Line 69 (original), 67 (patched)
> > <https://reviews.apache.org/r/62508/diff/1/?file=1832904#file1832904line69>
> >
> >     Nano-nit: Comment must start with a capital letter and end with a period.

Oh. Yeah. That's right.


> On Oct. 2, 2017, 12:26 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/windows/ip.hpp
> > Line 23 (original), 23-25 (patched)
> > <https://reviews.apache.org/r/62508/diff/1/?file=1832910#file1832910line23>
> >
> >     Hmm... a different style of comment?
> >     
> >     Since I prefer the one-line variety, I'll change it :)

I staged these one-by-one to make sure I'd fully converted them to one-line `// for ...`...
how I missed it, I don't know.


> On Oct. 2, 2017, 12:26 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 43-47 (patched)
> > <https://reviews.apache.org/r/62508/diff/1/?file=1832913#file1832913line49>
> >
> >     Going to plop down a comment here:
> >     ```
> >     // NOTE: These system headers must be included after `stout/windows.hpp`
> >     // as they may include `Windows.h`. See comments in `stout/windows.hpp`
> >     // for why this ordering is important.
> >     ```

Perfect.


- Andrew


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


On Sept. 22, 2017, 11:21 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62508/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:21 a.m.)
> 
> 
> Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7993
>     https://issues.apache.org/jira/browse/MESOS-7993
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch consolidates the inclusion of the ordering-dependent Windows
> system headers `WinSock2.h`, `WS2tcpip.h`, `Windows.h`, etc. For
> historical reasons, `Windows.h` will include `winsock.h`, the header for
> the Windows Sockets 1.1 library, which was last supported in Windows
> 2000. We use the Windows Sockets 2 library, which requires the
> `WinSock2.h` header, and this header must be included before
> `Windows.h`, otherwise symbol redefinition errors will occur. The
> `WS2tcpip.h` header includes the extensions to Windows Sockets 2, and
> any header which may include `Windows.h` must be included carefully.
> 
> It is simplest to consolidate the inclusion of these problematic system
> headers into `stout/windows.hpp`, and elsewhere include that with a
> comment as to which header specifically the file is requiring. Doing so
> will prevent incorrect ordering from being introduced.
> 
> Note that the erroneous inclusion of `winsock.h` was removed.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/ip.hpp a722fa47e05cf093e4ab4fed9d2824236dd5dd80 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 1c9dda0ba4653f970167e959139afb851682c6f8

>   3rdparty/stout/include/stout/os/windows/fd.hpp ae2db27154e694f319dafe2e04c01d55c42179de

>   3rdparty/stout/include/stout/os/windows/sendfile.hpp d50c89e4931b9109d7496fb4db496aea2ac7f830

>   3rdparty/stout/include/stout/os/windows/socket.hpp 18d2ecf560933d6a86cf945460b8611581b58cbb

>   3rdparty/stout/include/stout/windows.hpp 1d865f8fd23aba0198017f0bf4be8471cfb714ed 
>   3rdparty/stout/include/stout/windows/ip.hpp d7738f2fe9cb6818e5686dcb7dbb3cc73618f856

>   3rdparty/stout/include/stout/windows/mac.hpp 3ebf4e15e81e882d52a769811757f713a8ae65df

>   3rdparty/stout/include/stout/windows/net.hpp 364509f62f365eb5c1fd3ffe9aa38d1cb677c131

>   3rdparty/stout/include/stout/windows/os.hpp a70a61c702a422462872c0ec85f3c34e26a2e383

> 
> 
> Diff: https://reviews.apache.org/r/62508/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


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