[ https://issues.apache.org/jira/browse/ZOOKEEPER-2841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16090709#comment-16090709 ]
ASF GitHub Bot commented on ZOOKEEPER-2841: ------------------------------------------- Github user andschwa commented on the issue: https://github.com/apache/zookeeper/pull/311 @hanm this backports #306 to `branch-3.5`. I rebuilt and retested, all looks good. Since original branch came from `master` the version was already what I believe to be correct at `3.5.3`. > 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)