[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16091881#comment-16091881
 ] 

Michael Han commented on ZOOKEEPER-2841:
----------------------------------------

Committed to master. 
https://github.com/apache/zookeeper/commit/f60928787a908f358a64763f802a6d0371ad4404.
 Will resolve JIRA after committing ported patch to branch-3.4 and 3.5.

> ZooKeeper public include files leak porting changes
> ---------------------------------------------------
>
>                 Key: ZOOKEEPER-2841
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2841
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client
>         Environment: Windows 10 with Visual Studio 2017
>            Reporter: Andrew Schwartzmeyer
>            Assignee: Andrew Schwartzmeyer
>              Labels: windows
>
> The fundamental problem is that the port of the C client to Windows is now 
> close to six years old, with very few updates. This port leaks a lot of 
> changes that should be internal to ZooKeeper, and many of those changes are 
> simply no longer relevant. The correct thing to do is attempt to refactor the 
> Windows port for new versions of ZooKeeper, removing dead/unneeded porting 
> code, and moving dangerous porting code to C files instead of public headers.
> Two primary examples of this problem are 
> [ZOOKEEPER-2491|https://issues.apache.org/jira/browse/ZOOKEEPER-2491] and 
> [MESOS-7541|https://issues.apache.org/jira/browse/MESOS-7541].
> The first issue stems from this ancient porting code:
> {noformat}
> #define snprintf _snprintf
> {noformat}
>  in 
> [winconfig.h|https://github.com/apache/zookeeper/blob/ddf0364903bf7ac7cd25b2e1927f0d9d3c7203c4/src/c/include/winconfig.h#L179].
>  Newer versions of Windows C libraries define {{snprintf}} as a function, and 
> so it cannot be redefined.
> The second issue comes from this undocumented change:
> {noformat}
> #undef AF_INET6
> {noformat}
> again in 
> [winconfig.h|https://github.com/apache/zookeeper/blob/ddf0364903bf7ac7cd25b2e1927f0d9d3c7203c4/src/c/include/winconfig.h#L169]
>  which breaks any library that uses IPv6 and {{winsock2.h}}.
> Furthermore, the inclusion of the following defines and headers causes 
> terrible problems for consuming libraries, as they leak into ZooKeeper's 
> public headers:
> {noformat}
> #define _CRT_SECURE_NO_WARNINGS
> #define WIN32_LEAN_AND_MEAN
> #include <Windows.h>
> #include <Winsock2.h>
> #include <winstdint.h>
> #include <process.h>
> #include <ws2tcpip.h>
> {noformat}
> Depending on the order that a project includes or compiles files, this may or 
> may not cause {{WIN32_LEAN_AND_MEAN}} to become unexpectedly defined, and 
> {{windows.h}} to be unexpectedly included. This problem is exacberated by the 
> fact that the {{winsock2.h}} and {{windows.h}} headers are order-dependent 
> (if you read up on this, you'll see that defining {{WIN32_LEAN_AND_MEAN}} was 
> meant to work-around this).
> Going forward, porting changes should live next to where they are used, 
> preferably in source files, not header files, so they remain contained.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to