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

ASF GitHub Bot commented on ZOOKEEPER-2841:
-------------------------------------------

Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/306#discussion_r126290847
  
    --- Diff: src/c/CMakeLists.txt ---
    @@ -0,0 +1,220 @@
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you under the Apache License, Version 2.0 (the
    +# "License"); you may not use this file except in compliance
    +# with the License.  You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +
    +cmake_minimum_required(VERSION 3.6)
    +
    +project(zookeeper VERSION 3.5.2)
    --- End diff --
    
    The version here should be 3.5.3. I think you got 3.5.2 because when you 
start working on this, 3.5.3 was not released. 


> 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