Andrew Schwartzmeyer created ZOOKEEPER-2905:
-----------------------------------------------

             Summary: Don't include "config.h" in public headers
                 Key: ZOOKEEPER-2905
                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2905
             Project: ZooKeeper
          Issue Type: Bug
         Environment: Linux-ish environments.
            Reporter: Andrew Schwartzmeyer
            Assignee: Andrew Schwartzmeyer


In ZOOKEEPER-2841 I fixed the inclusion of project-specific porting changes 
that were included in the public headers, which then broke upstream projects 
(in my case, Mesos).

Unfortunately, I inadvertently created the exact same problem for Linux (or 
really any system that uses Autotools), and it wasn't evident until the build 
was coupled with another project with the same problem. More specifically, when 
including ZooKeeper (with my changes) in Mesos, and including Google's Glog in 
Mesos, and building both with Autotools (which we also support), both packages 
define the pre-processor macro {{PACKAGE_VERSION}}, and so so publicly. This is 
defined in {{config.h}} by Autotools, and is not a problem _unless included 
publicly_.

When refactoring, I saw two includes in {{zookeeper.h}} that instead of being 
guarded by e.g. {{#ifdef HAVE_SYS_SOCKET_H}} were guarded by {{#ifndef WIN32}}. 
Without realizing that I would create the exact same problem I was elsewhere 
fixing, I erroneously added {{#include "config.h"}} and guarded the includes 
"properly." But there is _very good reasons_ not to do this (explained above).

The patch to fix this is simple:

{noformat}
diff --git a/src/c/include/zookeeper.h b/src/c/include/zookeeper.h
index d20e70af4..b0bb09e3f 100644
--- a/src/c/include/zookeeper.h
+++ b/src/c/include/zookeeper.h
@@ -21,13 +21,9 @@

 #include <stdlib.h>

-#include "config.h"
-
-#ifdef HAVE_SYS_SOCKET_H
+/* we must not include config.h as a public header */
+#ifndef WIN32
 #include <sys/socket.h>
-#endif
-
-#ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
 #endif

diff --git a/src/c/src/zookeeper.c b/src/c/src/zookeeper.c
index 220c57dc4..9b837f227 100644
--- a/src/c/src/zookeeper.c
+++ b/src/c/src/zookeeper.c
@@ -24,6 +24,7 @@
 #define USE_IPV6
 #endif

+#include "config.h"
 #include <zookeeper.h>
 #include <zookeeper.jute.h>
 #include <proto.h>
{noformat}

I am opening pull requests in a few minutes to have this applied to branch 3.4 
and 3.5.

I'm sorry!



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to