[
https://issues.apache.org/jira/browse/ZOOKEEPER-2841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16078807#comment-16078807
]
ASF GitHub Bot commented on ZOOKEEPER-2841:
-------------------------------------------
GitHub user andschwa opened a pull request:
https://github.com/apache/zookeeper/pull/306
ZOOKEEPER-2841: ZooKeeper public include files leak porting changes
This PR includes the patches
[ZOOKEEPER-2756](https://issues.apache.org/jira/browse/ZOOKEEPER-2756) and
[ZOOKEEPER-2841](https://issues.apache.org/jira/browse/ZOOKEEPER-2841), and can
supplant PR #255.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andschwa/zookeeper ZOOKEEPER-2841
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/306.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #306
----
commit 187ce8acc1707a0dd20752b624a3fa5648706f97
Author: Andrew Schwartzmeyer <[email protected]>
Date: 2017-04-10T23:12:40Z
ZOOKEEPER-2756: Add CMake build system for better cross-platform support
This notably lacks Solaris and libtool support.
Almost everything else from Autotools has been ported,
including header/function/library checks, and all targets
(zookeeper, hashtable, cli, load_gen, and tests).
Both Linux and Windows are supported.
The primary work involved (other than the writing of `CMakeLists.txt`)
was transitioning the hand-written `winconfig.h` to an
auto-generated `config.h` file, and guarding code with `#ifdef
HAVE_FEATURE`. The `cmake_config.h.in` template was modeled after
the Autotools config file so that the feature guards share the same
names.
While `load_gen.c` looks at first glance as if it were ported to Windows,
it never actually was, so the erroneous `#include "win32port.h"` was
removed, and the target is not built on Windows.
There are existent warnings which this patch did not attempt to fix.
A guard was placed around `#define snprintf` in order to enable
compiling with modern versions of MSVC.
Fixed DLL_EXPORT and USE_STATIC_LIB redefinition.
As this commit deprecates (and breaks) the previously existing Visual
Studio solutions and projects, they've been removed.
Building tests on Windows is still not supported.
commit 22a378ae2a7c30657326b6a602b62116ab7c050a
Author: Andrew Schwartzmeyer <[email protected]>
Date: 2017-07-07T22:35:37Z
ZOOKEEPER-2841: ZooKeeper public include files leak porting changes
This patch refactors the Windows port of the C client. Notably, it moves
as much porting code as possible out of the publicly included
`winconfig.h` header and into the relevant source files. This also
removes `winstdint.h` as it has been replaced by `<stdint.h>`. It fixes
problems for upstream libraries by removing `#undef AF_INET6` and
`#include <windows.h>`.
Future work should resolve the `ACL` / `ZKACL` conflict, and potentially
remove `include/winconfig.h` altogether.
----
> 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)
