You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by eo...@apache.org on 2020/03/25 11:09:13 UTC
[zookeeper] branch master updated: ZOOKEEPER-3654: Incorrect
*_CFLAGS handling in Automake
This is an automated email from the ASF dual-hosted git repository.
eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 6486311 ZOOKEEPER-3654: Incorrect *_CFLAGS handling in Automake
6486311 is described below
commit 64863119bf92893f3c72350fd37005083b587327
Author: Damien Diederen <dd...@crosstwine.com>
AuthorDate: Wed Mar 25 12:08:54 2020 +0100
ZOOKEEPER-3654: Incorrect *_CFLAGS handling in Automake
The `Makefile.am` distributed with the C client defines some per-target `*_CFLAGS` and `*_CXXFLAGS` variables. These however, do not reference `AM_CFLAGS` (resp. AM_CXXFLAGS`, which means that some options (notably `-Wall`) are missing when building subsets of the code.
Dixit the [Automake docs](https://www.gnu.org/software/automake/manual/html_node/Program-and-Library-Variables.html):
> In compilations with per-target flags, the ordinary ‘AM_’ form of
> the flags variable is _not_ automatically included in the
> compilation (however, the user form of the variable _is_ included).
> So for instance, if you want the hypothetical ‘maude’ compilations
> to also use the value of ‘AM_CFLAGS’, you would need to write:
>
> maude_CFLAGS = ... your flags ... $(AM_CFLAGS)
Restoring the flags, however, causes compilation failures (in the library) and a slew of new warnings (in the tests) which had not been noticed because of the missing options.
This series of patches (all "tagged" ZOOKEEPER-3654) fix these warnings and errors before re-enabling `-Wall` and friends for all targets.
Author: Damien Diederen <dd...@crosstwine.com>
Reviewers: Enrico Olivelli <eo...@apache.org>, Andor Molnar <an...@apache.org>
Closes #1186 from ztzg/ZOOKEEPER-3654-incorrect-automake-flags
---
zookeeper-client/zookeeper-client-c/Makefile.am | 10 ++--
zookeeper-client/zookeeper-client-c/src/load_gen.c | 2 -
.../zookeeper-client-c/src/mt_adaptor.c | 3 +-
.../zookeeper-client-c/src/zookeeper.c | 2 +
.../zookeeper-client-c/tests/TestClient.cc | 25 ++++++----
.../zookeeper-client-c/tests/TestMulti.cc | 3 --
.../zookeeper-client-c/tests/TestReconfigServer.cc | 58 +++++++++++-----------
7 files changed, 53 insertions(+), 50 deletions(-)
diff --git a/zookeeper-client/zookeeper-client-c/Makefile.am b/zookeeper-client/zookeeper-client-c/Makefile.am
index 9c794a5..e42a353 100644
--- a/zookeeper-client/zookeeper-client-c/Makefile.am
+++ b/zookeeper-client/zookeeper-client-c/Makefile.am
@@ -60,7 +60,7 @@ libzookeeper_st_la_LDFLAGS = $(LIB_LDFLAGS) -export-symbols-regex $(EXPORT_SYMBO
if WANT_SYNCAPI
noinst_LTLIBRARIES += libzkmt.la
libzkmt_la_SOURCES =$(COMMON_SRC) src/mt_adaptor.c
-libzkmt_la_CFLAGS = -DTHREADED
+libzkmt_la_CFLAGS = $(AM_CFLAGS) -DTHREADED
libzkmt_la_LIBADD = -lm $(CLOCK_GETTIME_LIBS)
lib_LTLIBRARIES += libzookeeper_mt.la
@@ -80,11 +80,11 @@ bin_PROGRAMS += cli_mt load_gen
cli_mt_SOURCES = src/cli.c
cli_mt_LDADD = libzookeeper_mt.la $(SASL_LIB_LDFLAGS)
-cli_mt_CFLAGS = -DTHREADED
+cli_mt_CFLAGS = $(AM_CFLAGS) -DTHREADED
load_gen_SOURCES = src/load_gen.c
load_gen_LDADD = libzookeeper_mt.la
-load_gen_CFLAGS = -DTHREADED
+load_gen_CFLAGS = $(AM_CFLAGS) -DTHREADED
endif
@@ -134,14 +134,14 @@ TESTS_ENVIRONMENT = ZKROOT=${srcdir}/../.. \
CLASSPATH=$$CLASSPATH:$$CLOVER_HOME/lib/clover*.jar
nodist_zktest_st_SOURCES = $(TEST_SOURCES)
zktest_st_LDADD = libzkst.la libhashtable.la $(CPPUNIT_LIBS) $(OPENSSL_LIB_LDFLAGS) $(SASL_LIB_LDFLAGS) -ldl
-zktest_st_CXXFLAGS = -DUSE_STATIC_LIB $(CPPUNIT_CFLAGS) $(USEIPV6) $(SOLARIS_CPPFLAGS)
+zktest_st_CXXFLAGS = $(AM_CXXFLAGS) -DUSE_STATIC_LIB $(CPPUNIT_CFLAGS) $(SOLARIS_CPPFLAGS)
zktest_st_LDFLAGS = -shared $(SYMBOL_WRAPPERS) $(SOLARIS_LIB_LDFLAGS)
if WANT_SYNCAPI
check_PROGRAMS += zktest-mt
nodist_zktest_mt_SOURCES = $(TEST_SOURCES) tests/PthreadMocks.cc
zktest_mt_LDADD = libzkmt.la libhashtable.la -lpthread $(CPPUNIT_LIBS) $(OPENSSL_LIB_LDFLAGS) $(SASL_LIB_LDFLAGS) -ldl
- zktest_mt_CXXFLAGS = -DUSE_STATIC_LIB -DTHREADED $(CPPUNIT_CFLAGS) $(USEIPV6)
+ zktest_mt_CXXFLAGS = $(AM_CXXFLAGS) -DUSE_STATIC_LIB -DTHREADED $(CPPUNIT_CFLAGS) $(USEIPV6)
if SOLARIS
SHELL_SYMBOL_WRAPPERS_MT = cat ${srcdir}/tests/wrappers-mt.opt
SYMBOL_WRAPPERS_MT=$(SYMBOL_WRAPPERS) $(SHELL_SYMBOL_WRAPPERS_MT:sh)
diff --git a/zookeeper-client/zookeeper-client-c/src/load_gen.c b/zookeeper-client/zookeeper-client-c/src/load_gen.c
index 886fe1b..f25edcb 100644
--- a/zookeeper-client/zookeeper-client-c/src/load_gen.c
+++ b/zookeeper-client/zookeeper-client-c/src/load_gen.c
@@ -27,8 +27,6 @@
static zhandle_t *zh;
-static int shutdownThisThing=0;
-
// *****************************************************************************
//
static pthread_cond_t cond=PTHREAD_COND_INITIALIZER;
diff --git a/zookeeper-client/zookeeper-client-c/src/mt_adaptor.c b/zookeeper-client/zookeeper-client-c/src/mt_adaptor.c
index 38cced4..73b64c3 100644
--- a/zookeeper-client/zookeeper-client-c/src/mt_adaptor.c
+++ b/zookeeper-client/zookeeper-client-c/src/mt_adaptor.c
@@ -369,13 +369,14 @@ void *do_io(void *v)
fds[0].fd=adaptor_threads->self_pipe[0];
fds[0].events=POLLIN;
while(!zh->close_requested) {
- zh->io_count++;
struct timeval tv;
int fd;
int interest;
int timeout;
int maxfd=1;
+ zh->io_count++;
+
zookeeper_interest(zh, &fd, &interest, &tv);
if (fd != -1) {
fds[1].fd=fd;
diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c
index 2b8053e..a273225 100644
--- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c
+++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c
@@ -342,6 +342,7 @@ static int is_sasl_auth_in_progress(zhandle_t* zh)
#endif /* HAVE_CYRUS_SASL_H */
}
+#ifndef THREADED
/*
* abort due to the use of a sync api in a singlethreaded environment
*/
@@ -350,6 +351,7 @@ static void abort_singlethreaded(zhandle_t *zh)
LOG_ERROR(LOGCALLBACK(zh), "Sync completion used without threads");
abort();
}
+#endif /* THREADED */
static ssize_t zookeeper_send(zsock_t *fd, const void* buf, size_t len)
{
diff --git a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc
index a7d055f..2a6e992 100644
--- a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc
+++ b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc
@@ -325,9 +325,10 @@ public:
/** have a callback in the default watcher **/
static void default_zoo_watcher(zhandle_t *zzh, int type, int state, const char *path, void *context){
- int zrc = 0;
+ int zrc;
struct String_vector str_vec = {0, NULL};
zrc = zoo_wget_children(zzh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+ CPPUNIT_ASSERT(zrc == ZOK || zrc == ZCLOSING);
}
/** ZOOKEEPER-1057 This checks that the client connects to the second server when the first is not reachable **/
@@ -353,24 +354,28 @@ public:
/** this checks for a deadlock in calling zookeeper_close and calls from a default watcher that might get triggered just when zookeeper_close() is in progress **/
void testHangingClient() {
- int zrc = 0;
+ int zrc;
char buff[10] = "testall";
char path[512];
- watchctx_t *ctx;
+ watchctx_t *ctx = NULL;
struct String_vector str_vec = {0, NULL};
zhandle_t *zh = zookeeper_init(hostPorts, NULL, 10000, 0, ctx, 0);
sleep(1);
zrc = zoo_create(zh, "/mytest", buff, 10, &ZOO_OPEN_ACL_UNSAFE, 0, path, 512);
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zrc);
zrc = zoo_wget_children(zh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zrc);
zrc = zoo_create(zh, "/mytest/test1", buff, 10, &ZOO_OPEN_ACL_UNSAFE, 0, path, 512);
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zrc);
zrc = zoo_wget_children(zh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zrc);
zrc = zoo_delete(zh, "/mytest/test1", -1);
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zrc);
zookeeper_close(zh);
}
void testBadDescriptor() {
- int zrc = 0;
- watchctx_t *ctx;
+ watchctx_t *ctx = NULL;
zhandle_t *zh = zookeeper_init(hostPorts, NULL, 10000, 0, ctx, 0);
sleep(1);
zh->io_count = 0;
@@ -1039,8 +1044,8 @@ public:
CPPUNIT_ASSERT_EQUAL(zoo_get_log_callback(zk), &logMessageHandler);
// Log 10 messages and ensure all go to callback
- int expected = 10;
- for (int i = 0; i < expected; i++)
+ const std::size_t expected = 10;
+ for (std::size_t i = 0; i < expected; i++)
{
LOG_INFO(LOGCALLBACK(zk), "%s #%d", __FUNCTION__, i);
}
@@ -1057,12 +1062,12 @@ public:
// All the connection messages should have gone to the callback -- don't
// want this to be a maintenance issue so we're not asserting exact count
- int numBefore = logMessages.size();
+ std::size_t numBefore = logMessages.size();
CPPUNIT_ASSERT(numBefore != 0);
// Log 10 messages and ensure all go to callback
- int expected = 10;
- for (int i = 0; i < expected; i++)
+ const std::size_t expected = 10;
+ for (std::size_t i = 0; i < expected; i++)
{
LOG_INFO(LOGCALLBACK(zk), "%s #%d", __FUNCTION__, i);
}
diff --git a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
index 226e470..6dfeb96 100644
--- a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
+++ b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc
@@ -486,8 +486,6 @@ public:
watchctx_t ctx;
zhandle_t *zk = createClient(&ctx);
int sz = 512;
- char buf[sz];
- int blen;
char p1[sz];
p1[0] = '\0';
struct Stat stat;
@@ -573,7 +571,6 @@ public:
int sz = 512;
char p1[sz];
p1[0] = '\0';
- struct Stat s1;
rc = zoo_create(zk, "/multi0", "", 0, &ZOO_OPEN_ACL_UNSAFE, 0, p1, sz);
CPPUNIT_ASSERT_EQUAL((int)ZOK, rc);
diff --git a/zookeeper-client/zookeeper-client-c/tests/TestReconfigServer.cc b/zookeeper-client/zookeeper-client-c/tests/TestReconfigServer.cc
index c15774e..54810b6 100644
--- a/zookeeper-client/zookeeper-client-c/tests/TestReconfigServer.cc
+++ b/zookeeper-client/zookeeper-client-c/tests/TestReconfigServer.cc
@@ -49,12 +49,12 @@ class TestReconfigServer : public CPPUNIT_NS::TestFixture {
static const uint32_t NUM_SERVERS;
FILE* logfile_;
std::vector<ZooKeeperQuorumServer*> cluster_;
- int32_t getLeader();
- std::vector<int32_t> getFollowers();
+ std::size_t getLeader();
+ std::vector<std::size_t> getFollowers();
void parseConfig(char* buf, int len, std::vector<std::string>& servers,
std::string& version);
bool waitForConnected(zhandle_t* zh, uint32_t timeout_sec);
- zhandle_t* connectFollowers(std::vector<int32_t> &followers);
+ zhandle_t* connectFollowers(std::vector<std::size_t> &followers);
};
const uint32_t TestReconfigServer::NUM_SERVERS = 3;
@@ -84,15 +84,15 @@ setUp() {
void TestReconfigServer::
tearDown() {
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
delete cluster_[i];
}
cluster_.clear();
}
-int32_t TestReconfigServer::
+std::size_t TestReconfigServer::
getLeader() {
- for (int32_t i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (cluster_[i]->isLeader()) {
return i;
}
@@ -100,10 +100,10 @@ getLeader() {
return -1;
}
-std::vector<int32_t> TestReconfigServer::
+std::vector<std::size_t> TestReconfigServer::
getFollowers() {
- std::vector<int32_t> followers;
- for (int32_t i = 0; i < cluster_.size(); i++) {
+ std::vector<std::size_t> followers;
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (cluster_[i]->isFollower()) {
followers.push_back(i);
}
@@ -154,7 +154,7 @@ testRemoveFollower() {
char buf[len];
// get config from leader.
- int32_t leader = getLeader();
+ std::size_t leader = getLeader();
CPPUNIT_ASSERT(leader >= 0);
std::string host = cluster_[leader]->getHostPort();
zhandle_t* zk = zookeeper_init(host.c_str(), NULL, 10000, NULL, NULL, 0);
@@ -167,13 +167,13 @@ testRemoveFollower() {
// of the first NEWLEADER message, used as the initial version
CPPUNIT_ASSERT_EQUAL(std::string("100000000"), version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
cluster_[i]->getServerString()) != servers.end());
}
// remove a follower.
- std::vector<int32_t> followers = getFollowers();
+ std::vector<std::size_t> followers = getFollowers();
len = 1024;
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1,
(uint32_t)(followers.size()));
@@ -185,7 +185,7 @@ testRemoveFollower() {
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(std::string("100000002"), version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (i == followers[0]) {
continue;
}
@@ -201,7 +201,7 @@ testRemoveFollower() {
CPPUNIT_ASSERT_EQUAL((int)ZOK, rc);
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
cluster_[i]->getServerString()) != servers.end());
}
@@ -222,7 +222,7 @@ testNonIncremental() {
char buf[len];
// get config from leader.
- int32_t leader = getLeader();
+ std::size_t leader = getLeader();
CPPUNIT_ASSERT(leader >= 0);
std::string host = cluster_[leader]->getHostPort();
zhandle_t* zk = zookeeper_init(host.c_str(), NULL, 10000, NULL, NULL, 0);
@@ -236,18 +236,18 @@ testNonIncremental() {
// of the first NEWLEADER message, used as the initial version
CPPUNIT_ASSERT_EQUAL(std::string("100000000"), version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
cluster_[i]->getServerString()) != servers.end());
}
// remove a follower.
- std::vector<int32_t> followers = getFollowers();
+ std::vector<std::size_t> followers = getFollowers();
len = 1024;
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1,
(uint32_t)(followers.size()));
std::stringstream ss;
- for (int i = 1; i < followers.size(); i++) {
+ for (std::size_t i = 1; i < followers.size(); i++) {
ss << cluster_[followers[i]]->getServerString() << ",";
}
ss << cluster_[leader]->getServerString();
@@ -258,7 +258,7 @@ testNonIncremental() {
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(std::string("100000002"), version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (i == followers[0]) {
continue;
}
@@ -269,7 +269,7 @@ testNonIncremental() {
// add the follower back.
len = 1024;
ss.str("");
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
ss << cluster_[i]->getServerString() << ",";
}
rc = zoo_reconfig(zk, NULL, NULL, ss.str().c_str(), -1, buf, &len,
@@ -277,7 +277,7 @@ testNonIncremental() {
CPPUNIT_ASSERT_EQUAL((int)ZOK, rc);
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
cluster_[i]->getServerString()) != servers.end());
}
@@ -285,12 +285,12 @@ testNonIncremental() {
}
zhandle_t* TestReconfigServer::
-connectFollowers(std::vector<int32_t> &followers) {
+connectFollowers(std::vector<std::size_t> &followers) {
std::stringstream ss;
- int32_t leader = getLeader();
+ std::size_t leader = getLeader();
CPPUNIT_ASSERT(leader >= 0);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(followers.size()));
- for (int i = 0; i < followers.size(); i++) {
+ for (std::size_t i = 0; i < followers.size(); i++) {
ss << cluster_[followers[i]]->getHostPort() << ",";
}
ss << cluster_[leader]->getHostPort();
@@ -321,7 +321,7 @@ testRemoveConnectedFollower() {
// connect to a follower.
std::stringstream ss;
- std::vector<int32_t> followers = getFollowers();
+ std::vector<std::size_t> followers = getFollowers();
zhandle_t* zk = connectFollowers(followers);
CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
@@ -333,7 +333,7 @@ testRemoveConnectedFollower() {
CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (i == followers[0]) {
continue;
}
@@ -356,7 +356,7 @@ testReconfigFailureWithoutAuth() {
// connect to a follower.
std::stringstream ss;
- std::vector<int32_t> followers = getFollowers();
+ std::vector<std::size_t> followers = getFollowers();
zhandle_t* zk = connectFollowers(followers);
// remove the follower.
@@ -374,7 +374,7 @@ testReconfigFailureWithoutAuth() {
CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
parseConfig(buf, len, servers, version);
CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
- for (int i = 0; i < cluster_.size(); i++) {
+ for (std::size_t i = 0; i < cluster_.size(); i++) {
if (i == followers[0]) {
continue;
}
@@ -400,7 +400,7 @@ testReconfigFailureWithoutServerSuperuserPasswordConfigured() {
// connect to a follower.
std::stringstream ss;
- std::vector<int32_t> followers = getFollowers();
+ std::vector<std::size_t> followers = getFollowers();
zhandle_t* zk = connectFollowers(followers);
// remove the follower.