diff options
author | Damien Diederen <dd@crosstwine.com> | 2020-03-25 12:08:54 +0100 |
---|---|---|
committer | Enrico Olivelli <enrico.olivelli@diennea.com> | 2020-03-25 12:08:54 +0100 |
commit | 64863119bf92893f3c72350fd37005083b587327 (patch) | |
tree | d7077837a44e0a1242854d6e91c170c7468ce4c2 /zookeeper-client | |
parent | e87bad6774e7269ef21a156aff9dad089ef54794 (diff) | |
download | zookeeper-64863119bf92893f3c72350fd37005083b587327.tar.gz |
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 <eolivelli@apache.org>, Andor Molnar <andor@apache.org>
Closes #1186 from ztzg/ZOOKEEPER-3654-incorrect-automake-flags
Diffstat (limited to 'zookeeper-client')
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 9c794a5ef..e42a3539b 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 886fe1b3e..f25edcbe9 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 38cced425..73b64c391 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 2b8053e5a..a273225ff 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 a7d055fbe..2a6e99254 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 226e47082..6dfeb96e0 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 c15774e9e..54810b60c 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. |