From ebd5aef6dc92accb509b1cc67eaf72159f35cdfa Mon Sep 17 00:00:00 2001 From: Peter Iannucci Date: Fri, 22 Nov 2013 14:18:45 -0500 Subject: [PATCH] Clean-ups and fixes to compile with more warnings enabled and with g++. --- Makefile.osx | 32 +++++++++++++++++++++++--------- config.cc | 2 ++ config.h | 2 +- lock_protocol.h | 6 ++---- lock_tester.cc | 14 +++++++------- paxos.cc | 2 ++ paxos.h | 2 +- paxos_protocol.h | 2 +- rpc/connection.cc | 6 ++++-- rpc/connection.h | 4 +--- rpc/marshall_wrap.h | 4 ++-- rpc/poll_mgr.cc | 6 +++++- rpc/poll_mgr.h | 2 +- rpc/rpc.cc | 4 ++-- rpc/rpc_protocol.h | 6 +++--- rpc/rpctest.cc | 28 ++++++++++++++++------------ rsm.cc | 13 ++++++++++--- rsm.h | 2 +- rsm_client.h | 2 +- rsm_protocol.h | 6 +++--- threaded_log.h | 14 -------------- types.h | 14 ++++++++++++++ 22 files changed, 102 insertions(+), 71 deletions(-) diff --git a/Makefile.osx b/Makefile.osx index 74218df..25ea1af 100644 --- a/Makefile.osx +++ b/Makefile.osx @@ -1,14 +1,28 @@ -PEDANTRY = -Weverything -pedantic-errors -Werror -Wno-c++98-compat \ - -Wno-c++98-compat-pedantic -Wno-padded -Wno-missing-prototypes \ - -Wmissing-declarations -Wno-weak-vtables -Wno-global-constructors \ - -Wno-exit-time-destructors -pedantic -Wall -Wextra -Weffc++ -OPTFLAGS = -O3 -fno-omit-frame-pointer #-fsanitize=address ,thread,undefined -fsanitize-memory-track-origins -STDLIB = -stdlib=libc++ -#STDLIB = -CXX = clang++-mp-3.4 -#CXX = g++-mp-4.8 +USE_CLANG = 1 + +PEDANTRY = +STDLIB = +OPTFLAGS = -O3 #-fno-omit-frame-pointer -fsanitize=address ,thread,undefined -fsanitize-memory-track-origins CXXFLAGS = -std=c++11 -ggdb3 -MMD -I. $(STDLIB) $(PEDANTRY) $(OPTFLAGS) LDFLAGS = -std=c++11 $(STDLIB) $(OPTFLAGS) + +ifeq "$(USE_CLANG)" "1" + +PEDANTRY += \ + -Weverything -pedantic-errors -Werror -Wno-c++98-compat \ + -Wno-c++98-compat-pedantic -Wno-padded -Wno-global-constructors \ + -Wno-exit-time-destructors -pedantic -Wall -Wextra -Weffc++ +STDLIB += -stdlib=libc++ +CXX = clang++-mp-3.4 + +else + +PEDANTRY += -pedantic -Wall -Wextra -fno-default-inline -Werror +STDLIB += -pthread +CXX = g++-mp-4.8 + +endif + CC := $(CXX) EXTRA_TARGETS = signatures diff --git a/config.cc b/config.cc index 7df1bbc..cd06e1b 100644 --- a/config.cc +++ b/config.cc @@ -33,6 +33,8 @@ // all views, the other nodes can bring this re-joined node up to // date. +config_view_change::~config_view_change() {} + config::config(const string & _first, const string & _me, config_view_change *_vc) : my_view_id(0), first(_first), me(_me), vc(_vc), paxos(this, me == _first, me, me) diff --git a/config.h b/config.h index 895de1b..26a612d 100644 --- a/config.h +++ b/config.h @@ -7,7 +7,7 @@ class config_view_change { public: virtual void commit_change(unsigned view_id) = 0; - virtual ~config_view_change() {} + virtual ~config_view_change(); }; class config : public paxos_change { diff --git a/lock_protocol.h b/lock_protocol.h index 5589c07..8cccebe 100644 --- a/lock_protocol.h +++ b/lock_protocol.h @@ -1,5 +1,3 @@ -// lock protocol - #ifndef lock_protocol_h #define lock_protocol_h @@ -16,7 +14,7 @@ namespace lock_protocol { REMOTE_PROCEDURE(1, acquire, (int &, lockid_t, callback_t, xid_t)); REMOTE_PROCEDURE(2, release, (int &, lockid_t, callback_t, xid_t)); REMOTE_PROCEDURE(3, stat, (int &, lockid_t, callback_t)); -}; +} namespace rlock_protocol { using lockid_t = lock_protocol::lockid_t; @@ -25,5 +23,5 @@ namespace rlock_protocol { REMOTE_PROCEDURE_BASE(0x8000); REMOTE_PROCEDURE(1, revoke, (int &, lockid_t, xid_t)); REMOTE_PROCEDURE(2, retry, (int &, lockid_t, xid_t)); -}; +} #endif diff --git a/lock_tester.cc b/lock_tester.cc index f535d8f..5e615ca 100644 --- a/lock_tester.cc +++ b/lock_tester.cc @@ -22,7 +22,7 @@ static lock_protocol::lockid_t c = "3"; static int ct[256]; static mutex count_mutex; -void check_grant(lock_protocol::lockid_t lid) { +static void check_grant(lock_protocol::lockid_t lid) { lock ml(count_mutex); int x = lid[0] & 0x0f; if (ct[x] != 0) { @@ -32,7 +32,7 @@ void check_grant(lock_protocol::lockid_t lid) { ct[x] += 1; } -void check_release(lock_protocol::lockid_t lid) { +static void check_release(lock_protocol::lockid_t lid) { lock ml(count_mutex); int x = lid[0] & 0x0f; if (ct[x] != 1) { @@ -42,7 +42,7 @@ void check_release(lock_protocol::lockid_t lid) { ct[x] -= 1; } -void test1(void) { +static void test1(void) { LOG_NONMEMBER("acquire a release a acquire a release a"); lc[0]->acquire(a); check_grant(a); @@ -64,7 +64,7 @@ void test1(void) { check_release(a); } -void test2(int i) { +static void test2(int i) { LOG_NONMEMBER("test2: client " << i << " acquire a release a"); lc[i]->acquire(a); LOG_NONMEMBER("test2: client " << i << " acquire done"); @@ -76,7 +76,7 @@ void test2(int i) { LOG_NONMEMBER("test2: client " << i << " release done"); } -void test3(int i) { +static void test3(int i) { LOG_NONMEMBER("test3: client " << i << " acquire a release a concurrent"); for (int j = 0; j < 10; j++) { lc[i]->acquire(a); @@ -87,7 +87,7 @@ void test3(int i) { } } -void test4(int i) { +static void test4(int i) { LOG_NONMEMBER("test4: thread " << i << " acquire a release a concurrent; same clnt"); for (int j = 0; j < 10; j++) { lc[0]->acquire(a); @@ -98,7 +98,7 @@ void test4(int i) { } } -void test5(int i) { +static void test5(int i) { LOG_NONMEMBER("test5: client " << i << " acquire a release a concurrent; same and diff clnt"); for (int j = 0; j < 10; j++) { if (i < 5) lc[0]->acquire(a); diff --git a/paxos.cc b/paxos.cc index dad5ecf..4bba25d 100644 --- a/paxos.cc +++ b/paxos.cc @@ -1,6 +1,8 @@ #include "paxos.h" #include "handle.h" +paxos_change::~paxos_change() {} + bool isamember(const node_t & m, const nodes_t & nodes) { return find(nodes.begin(), nodes.end(), m) != nodes.end(); } diff --git a/paxos.h b/paxos.h index f1123e6..2ddf583 100644 --- a/paxos.h +++ b/paxos.h @@ -14,7 +14,7 @@ using value_t = paxos_protocol::value_t; class paxos_change { public: virtual void paxos_commit(unsigned instance, const value_t & v) = 0; - virtual ~paxos_change() {} + virtual ~paxos_change(); }; extern bool isamember(const node_t & m, const nodes_t & nodes); diff --git a/paxos_protocol.h b/paxos_protocol.h index c61e2eb..30a3d2c 100644 --- a/paxos_protocol.h +++ b/paxos_protocol.h @@ -33,7 +33,7 @@ namespace paxos_protocol { REMOTE_PROCEDURE(2, acceptreq, (bool &, node_t, unsigned, prop_t, value_t)); REMOTE_PROCEDURE(3, decidereq, (int &, node_t, unsigned, value_t)); REMOTE_PROCEDURE(4, heartbeat, (int &, string, unsigned)); -}; +} MARSHALLABLE_STRUCT(paxos_protocol::prepareres) diff --git a/rpc/connection.cc b/rpc/connection.cc index 4e49305..c4edbf6 100644 --- a/rpc/connection.cc +++ b/rpc/connection.cc @@ -6,6 +6,8 @@ #include #include "marshall.h" +connection_delegate::~connection_delegate() {} + connection::connection(connection_delegate * delegate, socket_t && f1, int l1) : fd(move(f1)), delegate_(delegate), lossy_(l1) { @@ -204,7 +206,7 @@ connection_listener::connection_listener(connection_delegate * delegate, in_port tcp_.setsockopt(SOL_SOCKET, SO_RCVTIMEO, timeval{0, 50000}); tcp_.setsockopt(SOL_SOCKET, SO_SNDTIMEO, timeval{0, 50000}); - sockaddr_in sin{}; // zero initialize + sockaddr_in sin = sockaddr_in(); // zero initialize sin.sin_family = AF_INET; sin.sin_port = hton(port); @@ -237,7 +239,7 @@ void connection_listener::read_cb(int) { int s1 = accept(tcp_, (sockaddr *)&sin, &slen); if (s1 < 0) { perror("connection_listener::accept_conn error"); - throw thread_exit_exception(); + throw runtime_error("connection listener failure"); } IF_LEVEL(2) LOG("accept_loop got connection fd=" << s1 << " " << inet_ntoa(sin.sin_addr) << ":" << ntoh(sin.sin_port)); diff --git a/rpc/connection.h b/rpc/connection.h index 3133299..1bcb7b6 100644 --- a/rpc/connection.h +++ b/rpc/connection.h @@ -9,14 +9,12 @@ constexpr size_t size_t_max = numeric_limits::max(); -class thread_exit_exception : exception {}; - class connection; class connection_delegate { public: virtual bool got_pdu(const shared_ptr & c, const string & b) = 0; - virtual ~connection_delegate() {} + virtual ~connection_delegate(); }; class connection : private aio_callback, public enable_shared_from_this { diff --git a/rpc/marshall_wrap.h b/rpc/marshall_wrap.h index fa55f17..4ba7a20 100644 --- a/rpc/marshall_wrap.h +++ b/rpc/marshall_wrap.h @@ -43,7 +43,7 @@ struct VerifyOnFailure { // One for function pointers... template -typename enable_if::value, RV>::type +typename enable_if::value, RV>::type inline invoke(RV, F f, void *, R & r, args_type & t, tuple_indices) { return f(r, move(get(t))...); } @@ -51,7 +51,7 @@ invoke(RV, F f, void *, R & r, args_type & t, tuple_indices) { // And one for pointers to member functions... template -typename enable_if::value, RV>::type +typename enable_if::value, RV>::type inline invoke(RV, F f, C *c, R & r, args_type & t, tuple_indices) { return (c->*f)(r, move(get(t))...); } diff --git a/rpc/poll_mgr.cc b/rpc/poll_mgr.cc index dc42274..2598249 100644 --- a/rpc/poll_mgr.cc +++ b/rpc/poll_mgr.cc @@ -7,6 +7,8 @@ #include #endif +aio_callback::~aio_callback() {} + poll_mgr poll_mgr::shared_mgr; class wait_manager { @@ -14,9 +16,11 @@ class wait_manager { virtual void watch_fd(int fd, poll_flag flag) = 0; virtual bool unwatch_fd(int fd, poll_flag flag) = 0; virtual void wait_ready(vector & readable, vector & writable) = 0; - virtual ~wait_manager() noexcept {} + virtual ~wait_manager() noexcept; }; +wait_manager::~wait_manager() noexcept {} + class SelectAIO : public wait_manager { public : SelectAIO(); diff --git a/rpc/poll_mgr.h b/rpc/poll_mgr.h index bd451cf..44ac9f8 100644 --- a/rpc/poll_mgr.h +++ b/rpc/poll_mgr.h @@ -17,7 +17,7 @@ class aio_callback { public: virtual void read_cb(int fd) = 0; virtual void write_cb(int fd) = 0; - virtual ~aio_callback() {} + virtual ~aio_callback(); }; class poll_mgr { diff --git a/rpc/rpc.cc b/rpc/rpc.cc index 7937785..6985498 100644 --- a/rpc/rpc.cc +++ b/rpc/rpc.cc @@ -99,7 +99,7 @@ int rpcc::bind(milliseconds to) { IF_LEVEL(2) LOG("bind " << inet_ntoa(dst_.sin_addr) << " failed " << ret); } return ret; -}; +} // Cancel all outstanding calls void rpcc::cancel(void) { @@ -537,7 +537,7 @@ static sockaddr_in make_sockaddr(const string & hostandport) { port = hostandport.substr(colon+1); } - sockaddr_in dst{}; // zero initialize + sockaddr_in dst = sockaddr_in(); // zero initialize dst.sin_family = AF_INET; struct in_addr a{inet_addr(host.c_str())}; diff --git a/rpc/rpc_protocol.h b/rpc/rpc_protocol.h index 65b7523..b2fc346 100644 --- a/rpc/rpc_protocol.h +++ b/rpc/rpc_protocol.h @@ -48,12 +48,12 @@ namespace rpc_protocol { const size_t DEFAULT_RPC_SZ = 1024; // size of initial buffer allocation const size_t MAX_PDU = 10<<20; // maximum PDF is 10M -#define REMOTE_PROCEDURE_BASE(_base_) static constexpr rpc_protocol::proc_id_t base = _base_; -#define REMOTE_PROCEDURE(_offset_, _name_, _args_) static constexpr rpc_protocol::proc_t _name_{base + _offset_}; +#define REMOTE_PROCEDURE_BASE(_base_) static constexpr rpc_protocol::proc_id_t base = _base_ +#define REMOTE_PROCEDURE(_offset_, _name_, _args_) static constexpr rpc_protocol::proc_t _name_{base + _offset_} REMOTE_PROCEDURE_BASE(0); REMOTE_PROCEDURE(1, bind, (nonce_t &)); // handler number reserved for bind -}; +} ENDIAN_SWAPPABLE(rpc_protocol::request_header) ENDIAN_SWAPPABLE(rpc_protocol::reply_header) diff --git a/rpc/rpctest.cc b/rpc/rpctest.cc index d4f53f4..f7df5fe 100644 --- a/rpc/rpctest.cc +++ b/rpc/rpctest.cc @@ -35,7 +35,7 @@ namespace srv_protocol { REMOTE_PROCEDURE(23, fast, (int &, int)); REMOTE_PROCEDURE(24, slow, (int &, int)); REMOTE_PROCEDURE(25, bigrep, (string &, size_t)); -}; +} // a handler. a and b are arguments, r is the result. // there can be multiple arguments but only one result. @@ -67,7 +67,7 @@ int srv::handle_bigrep(string & r, const size_t len) { static srv service; -void startserver() { +static void startserver() { server = new rpcs(port); server->reg(srv_protocol::_22, &srv::handle_22, &service); server->reg(srv_protocol::fast, &srv::handle_fast, &service); @@ -76,7 +76,7 @@ void startserver() { server->start(); } -void testmarshall() { +static void testmarshall() { marshall m; rpc_protocol::request_header rh{1,2,3,4,5}; m.pack_header(rh); @@ -85,13 +85,15 @@ void testmarshall() { unsigned long long l = 1223344455L; size_t sz = 101010101; string s = "hallo...."; + string bin("\x00\x00\x00\x00\x00\x00\x00\x40\x00\x00\x7f\xe5", 12); m << i; m << l; m << s; m << sz; + m << bin; string b = m; - VERIFY(b.size() == rpc_protocol::RPC_HEADER_SZ+sizeof(i)+sizeof(l)+s.size()+sizeof(int)+sizeof(uint32_t)); + VERIFY(b.size() == rpc_protocol::RPC_HEADER_SZ+sizeof(i)+sizeof(l)+sizeof(uint32_t)+s.size()+sizeof(uint32_t)+sizeof(uint32_t)+bin.size()); unmarshall un(b, true); rpc_protocol::request_header rh1; @@ -100,16 +102,18 @@ void testmarshall() { int i1; unsigned long long l1; string s1; + string bin1; size_t sz1; un >> i1; un >> l1; un >> s1; un >> sz1; + un >> bin1; VERIFY(un.okdone()); - VERIFY(i1==i && l1==l && s1==s && sz1==sz); + VERIFY(i1==i && l1==l && s1==s && sz1==sz && bin1==bin); } -void client1(size_t cl) { +static void client1(size_t cl) { // test concurrency. size_t which_cl = cl % NUM_CL; @@ -142,7 +146,7 @@ void client1(size_t cl) { } } -void client2(size_t cl) { +static void client2(size_t cl) { size_t which_cl = cl % NUM_CL; time_t t1; @@ -158,7 +162,7 @@ void client2(size_t cl) { } } -void client3(void *xx) { +static void client3(void *xx) { rpcc *c = (rpcc *) xx; for(int i = 0; i < 4; i++){ @@ -168,7 +172,7 @@ void client3(void *xx) { } } -void simple_tests(rpcc *c) { +static void simple_tests(rpcc *c) { cout << "simple_tests" << endl; // an RPC call to procedure #22. // rpcc::call() looks at the argument types to decide how @@ -219,7 +223,7 @@ void simple_tests(rpcc *c) { cout << "simple_tests OK" << endl; } -void concurrent_test(size_t nt) { +static void concurrent_test(size_t nt) { // create threads that make lots of calls in parallel, // to test thread synchronization for concurrent calls // and dispatches. @@ -236,7 +240,7 @@ void concurrent_test(size_t nt) { cout << " OK" << endl; } -void lossy_test() { +static void lossy_test() { cout << "start lossy_test ..."; VERIFY(setenv("RPC_LOSSY", "5", 1) == 0); @@ -265,7 +269,7 @@ void lossy_test() { VERIFY(setenv("RPC_LOSSY", "0", 1) == 0); } -void failure_test() { +static void failure_test() { rpcc *client1; rpcc *client = clients[0]; diff --git a/rsm.cc b/rsm.cc index c766145..de37b5d 100644 --- a/rsm.cc +++ b/rsm.cc @@ -83,6 +83,8 @@ #include "rsm_client.h" #include +rsm_state_transfer::~rsm_state_transfer() {} + rsm::rsm(const string & _first, const string & _me) : primary(_first) { cfg = unique_ptr(new config(_first, _me, this)); @@ -347,10 +349,15 @@ rsm_client_protocol::status rsm::client_invoke(string & r, rpc_protocol::proc_id partition1(rsm_mutex_lock); } } + LOG(setfill('0') << setw(2) << hex; + for (size_t i=0; i - int call(rpc_protocol::proc_t

proc, R & r, const Args & ...a1) { + inline int call(rpc_protocol::proc_t

proc, R & r, const Args & ...a1) { static_assert(is_valid_call::value, "RSM method invoked with incorrect argument types"); return call_m(proc.id, r, marshall(a1...)); } diff --git a/rsm_protocol.h b/rsm_protocol.h index 9cd60bd..9e53430 100644 --- a/rsm_protocol.h +++ b/rsm_protocol.h @@ -9,7 +9,7 @@ namespace rsm_client_protocol { REMOTE_PROCEDURE_BASE(0x9000); REMOTE_PROCEDURE(1, invoke, (string &, rpc_protocol::proc_id_t, string)); REMOTE_PROCEDURE(2, members, (vector &, int)); -}; +} struct viewstamp { unsigned int vid; @@ -37,7 +37,7 @@ namespace rsm_protocol { REMOTE_PROCEDURE(2, transferreq, (transferres &, string, viewstamp, unsigned)); REMOTE_PROCEDURE(3, transferdonereq, (int &, string, unsigned)); REMOTE_PROCEDURE(4, joinreq, (string &, string, viewstamp)); -}; +} MARSHALLABLE_STRUCT(rsm_protocol::transferres) @@ -46,6 +46,6 @@ namespace rsm_test_protocol { REMOTE_PROCEDURE_BASE(0x12000); REMOTE_PROCEDURE(1, net_repair, (status &, int)); REMOTE_PROCEDURE(2, breakpoint, (status &, int)); -}; +} #endif diff --git a/threaded_log.h b/threaded_log.h index 903b0fa..bccd813 100644 --- a/threaded_log.h +++ b/threaded_log.h @@ -10,20 +10,6 @@ extern map instance_name_map; extern int next_instance_num; extern char log_thread_prefix; -namespace std { - // Sticking this in std:: makes it possible for ostream_iterator to use it. - template - ostream & operator<<(ostream & o, const pair & d) { - return o << "<" << d.first << "," << d.second << ">"; - } -} - -template -typename enable_if::value && !is_same::value, ostream>::type & -operator<<(ostream & o, const A & a) { - return o << "[" << implode(a, ", ") << "]"; -} - #define LOG_PREFIX { \ auto _thread_ = this_thread::get_id(); \ int _tid_ = thread_name_map[_thread_]; \ diff --git a/types.h b/types.h index 888cd68..0241bd3 100644 --- a/types.h +++ b/types.h @@ -75,6 +75,9 @@ using std::weak_ptr; using std::mutex; using lock = std::unique_lock; +#include +using std::runtime_error; + #include using std::ostringstream; using std::istringstream; @@ -138,6 +141,11 @@ template constexpr inline E to_enum(enum_type_t value) noexcept { // string manipulation +template +ostream & operator<<(ostream & o, const pair & d) { + return o << "<" << d.first << "," << d.second << ">"; +} + template inline typename enable_if::value, string>::type implode(const C & v, string delim=" ") { @@ -162,6 +170,12 @@ inline vector explode(const string & s, string delim=" ") { return out; } +template +typename enable_if::value && !is_same::value, ostream>::type & +operator<<(ostream & o, const A & a) { + return o << "[" << implode(a, ", ") << "]"; +} + #include "verify.h" #include "threaded_log.h" -- 1.7.9.5