[
https://issues.apache.org/jira/browse/ZOOKEEPER-2905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16183435#comment-16183435
]
ASF GitHub Bot commented on ZOOKEEPER-2905:
-------------------------------------------
Github user andschwa commented on the issue:
https://github.com/apache/zookeeper/pull/382
Thanks!
> Don't include `config.h` in `zookeeper.h`
> -----------------------------------------
>
> 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
> Fix For: 3.5.4, 3.6.0, 3.4.11
>
>
> 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)
