Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-2841

ZooKeeper public include files leak porting changes


    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.11, 3.5.4, 3.6.0
    • Component/s: c client
    • Labels:
    • Environment:

      Windows 10 with Visual Studio 2017

    • Release Note:
      cmake is added to replace the existing hardcoded (and outdated) visual studio solutions for windows platform.


      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 and MESOS-7541.

      The first issue stems from this ancient porting code:

      #define snprintf _snprintf

      in winconfig.h. 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:

      #undef AF_INET6

      again in winconfig.h 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:

      #define WIN32_LEAN_AND_MEAN
      #include <Windows.h>
      #include <Winsock2.h>
      #include <winstdint.h>
      #include <process.h>
      #include <ws2tcpip.h>

      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.


          Issue Links



              • Assignee:
                andschwa Andrew Schwartzmeyer
                andschwa Andrew Schwartzmeyer
              • Votes:
                0 Vote for this issue
                3 Start watching this issue


                • Created: