[
https://issues.apache.org/jira/browse/ZOOKEEPER-2841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16091905#comment-16091905
]
ASF GitHub Bot commented on ZOOKEEPER-2841:
-------------------------------------------
Github user andschwa commented on the issue:
https://github.com/apache/zookeeper/pull/313
@hanm this backports #306 to `branch-3.4`. I think I caught everything, but
do give it a review. The build changed a bit between 3.4 and 3.5, so I had to
remove some files from `CMakeLists.txt`, and I set the version to `3.4.10`.
Then there were some fixes I had to do a bit differently (moved the `#define
random` into `zookeeper.c` since `addrvec.c` wasn't created yet, reapply the
changes I made to `zookeeper_interest` by hand, and one trivial merge conflict
in `recordio.h`).
I build and ran the tests on Linux, all passing save:
```
1: Zookeeper_simpleSystem::testAsyncWatcherAutoResetterminate called after
throwing an instance of 'CppUnit::Exception'
1: what(): equality assertion failed
1: - Expected: -101
1: - Actual : -4
```
which is odd. Is it a flaky test, or very machine dependent?
I built on Windows, and it all compiled and linked successfully. If you
want, I can integration test it. I probably should anyway so I can move Mesos
on Windows back to down to ZK 3.4 instead of 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)
