zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ph...@apache.org
Subject zookeeper git commit: ZOOKEEPER-2905: Don't include `config.h` in `zookeeper.h`
Date Wed, 27 Sep 2017 22:21:48 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.5 313f1eb08 -> 6792ee994


ZOOKEEPER-2905: Don't include `config.h` in `zookeeper.h`

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

Unfortunately, I inadvertently created a very similar problem, and it
wasn't evident until the build was coupled with another project with the
same bug. More specifically, when including ZooKeeper in Mesos, and
including Google's Glog in Mesos, both projects define the macros
`VERSION`, `PACKAGE_VERSION`, and `PACKAGE_TARNAME`, and do so publicly.
This is commonly defined in `config.h` by Autotools (and by CMake for
ZooKeeper for compatibility), and is not a problem unless included
publicly, such as in `zookeeper.h`, and by more than one project.

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`. I erroneously added `#include "config.h"` and guarded
the includes "properly" with a feature guard. However, configuration
files such as `config.h` and `winconfig.h` etc. must never be included
in publicly in `zookeeper.h`, for the reasons given above.

This patch reverts the bug, and instead includes `config.h` in
`zookeeper.c`, where it is not exposed to other projects.

Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>

Reviewers: Patrick Hunt <phunt@apache.org>

Closes #382 from andschwa/ZOOKEEPER-2905-3.5

Change-Id: I483edd572a1a0c54a2ce41aa3bd64b606c9910e6


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/6792ee99
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/6792ee99
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/6792ee99

Branch: refs/heads/branch-3.5
Commit: 6792ee994ce8807d6cbbcfae37154edf43c07cd9
Parents: 313f1eb
Author: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
Authored: Wed Sep 27 15:21:04 2017 -0700
Committer: Patrick Hunt <phunt@apache.org>
Committed: Wed Sep 27 15:21:04 2017 -0700

----------------------------------------------------------------------
 src/c/include/zookeeper.h | 8 ++------
 src/c/src/zookeeper.c     | 1 +
 2 files changed, 3 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/6792ee99/src/c/include/zookeeper.h
----------------------------------------------------------------------
diff --git a/src/c/include/zookeeper.h b/src/c/include/zookeeper.h
index dc2e0c9..af601f1 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
 

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/6792ee99/src/c/src/zookeeper.c
----------------------------------------------------------------------
diff --git a/src/c/src/zookeeper.c b/src/c/src/zookeeper.c
index ca2e75b..8c77243 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>


Mime
View raw message