Repository: thrift Updated Branches: refs/heads/master 3fa194048 -> 36200904e
THRIFT-3943: resolve some high severity outstanding defects identified by coverity scan Clients: C++, Lua Patch: James E. King, III <jim.k...@simplivity.com> This closes #1109 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/36200904 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/36200904 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/36200904 Branch: refs/heads/master Commit: 36200904e78f11dd0ca2d751a9b35bb54790267b Parents: 3fa1940 Author: James E. King, III <jim.k...@simplivity.com> Authored: Wed Oct 5 14:47:18 2016 -0400 Committer: Jens Geyer <je...@apache.org> Committed: Thu Oct 13 22:59:20 2016 +0200 ---------------------------------------------------------------------- lib/cpp/src/thrift/concurrency/FunctionRunner.h | 4 ++-- lib/cpp/src/thrift/transport/TSocket.cpp | 21 +++++++++----------- lib/cpp/src/thrift/transport/TSocket.h | 18 ++++++++--------- lib/cpp/test/TFileTransportTest.cpp | 12 +++++------ lib/cpp/test/concurrency/ThreadFactoryTests.h | 2 +- lib/cpp/test/concurrency/ThreadManagerTests.h | 2 +- lib/cpp/test/concurrency/TimerManagerTests.h | 1 + lib/cpp/test/processor/EventLog.cpp | 20 +++++++++++-------- lib/lua/src/usocket.c | 7 ++++--- 9 files changed, 45 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/src/thrift/concurrency/FunctionRunner.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/concurrency/FunctionRunner.h b/lib/cpp/src/thrift/concurrency/FunctionRunner.h index b776794..9c085c0 100644 --- a/lib/cpp/src/thrift/concurrency/FunctionRunner.h +++ b/lib/cpp/src/thrift/concurrency/FunctionRunner.h @@ -81,12 +81,12 @@ public: * execute the given callback. Note that the 'void*' return value is ignored. */ FunctionRunner(PthreadFuncPtr func, void* arg) - : func_(apache::thrift::stdcxx::bind(pthread_func_wrapper, func, arg)) {} + : func_(apache::thrift::stdcxx::bind(pthread_func_wrapper, func, arg)), intervalMs_(-1) {} /** * Given a generic callback, this FunctionRunner will execute it. */ - FunctionRunner(const VoidFunc& cob) : func_(cob) {} + FunctionRunner(const VoidFunc& cob) : func_(cob), intervalMs_(-1) {} /** * Given a bool foo(...) type callback, FunctionRunner will execute http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/src/thrift/transport/TSocket.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/transport/TSocket.cpp b/lib/cpp/src/thrift/transport/TSocket.cpp index bc1bbdd..e1c106a 100644 --- a/lib/cpp/src/thrift/transport/TSocket.cpp +++ b/lib/cpp/src/thrift/transport/TSocket.cpp @@ -81,8 +81,8 @@ using namespace std; TSocket::TSocket(const string& host, int port) : host_(host), port_(port), - path_(""), socket_(THRIFT_INVALID_SOCKET), + peerPort_(0), connTimeout_(0), sendTimeout_(0), recvTimeout_(0), @@ -94,10 +94,10 @@ TSocket::TSocket(const string& host, int port) } TSocket::TSocket(const string& path) - : host_(""), - port_(0), + : port_(0), path_(path), socket_(THRIFT_INVALID_SOCKET), + peerPort_(0), connTimeout_(0), sendTimeout_(0), recvTimeout_(0), @@ -110,10 +110,9 @@ TSocket::TSocket(const string& path) } TSocket::TSocket() - : host_(""), - port_(0), - path_(""), + : port_(0), socket_(THRIFT_INVALID_SOCKET), + peerPort_(0), connTimeout_(0), sendTimeout_(0), recvTimeout_(0), @@ -126,10 +125,9 @@ TSocket::TSocket() } TSocket::TSocket(THRIFT_SOCKET socket) - : host_(""), - port_(0), - path_(""), + : port_(0), socket_(socket), + peerPort_(0), connTimeout_(0), sendTimeout_(0), recvTimeout_(0), @@ -148,10 +146,9 @@ TSocket::TSocket(THRIFT_SOCKET socket) } TSocket::TSocket(THRIFT_SOCKET socket, boost::shared_ptr<THRIFT_SOCKET> interruptListener) - : host_(""), - port_(0), - path_(""), + : port_(0), socket_(socket), + peerPort_(0), interruptListener_(interruptListener), connTimeout_(0), sendTimeout_(0), http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/src/thrift/transport/TSocket.h ---------------------------------------------------------------------- diff --git a/lib/cpp/src/thrift/transport/TSocket.h b/lib/cpp/src/thrift/transport/TSocket.h index 9f0074d..aa18c31 100644 --- a/lib/cpp/src/thrift/transport/TSocket.h +++ b/lib/cpp/src/thrift/transport/TSocket.h @@ -272,15 +272,6 @@ protected: /** Host to connect to */ std::string host_; - /** Peer hostname */ - std::string peerHost_; - - /** Peer address */ - std::string peerAddress_; - - /** Peer port */ - int peerPort_; - /** Port number to connect on */ int port_; @@ -290,6 +281,15 @@ protected: /** Underlying socket handle */ THRIFT_SOCKET socket_; + /** Peer hostname */ + std::string peerHost_; + + /** Peer address */ + std::string peerAddress_; + + /** Peer port */ + int peerPort_; + /** * A shared socket pointer that will interrupt a blocking read if data * becomes available on it http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/test/TFileTransportTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/TFileTransportTest.cpp b/lib/cpp/test/TFileTransportTest.cpp index 8551b78..82e84e8 100644 --- a/lib/cpp/test/TFileTransportTest.cpp +++ b/lib/cpp/test/TFileTransportTest.cpp @@ -189,7 +189,7 @@ BOOST_AUTO_TEST_CASE(test_destructor) { unsigned int num_over = 0; for (unsigned int n = 0; n < NUM_ITERATIONS; ++n) { - ftruncate(f.getFD(), 0); + BOOST_CHECK_EQUAL(0, ftruncate(f.getFD(), 0)); TFileTransport* transport = new TFileTransport(f.getPath()); @@ -392,21 +392,21 @@ void parse_args(int argc, char* argv[]) { #ifdef BOOST_TEST_DYN_LINK static int myArgc = 0; static char **myArgv = NULL; - + bool init_unit_test_suite() { boost::unit_test::framework::master_test_suite().p_name.value = "TFileTransportTest"; - + // Parse arguments parse_args(myArgc,myArgv); return true; } - + int main( int argc, char* argv[] ) { myArgc = argc; myArgv = argv; return ::boost::unit_test::unit_test_main(&init_unit_test_suite,argc,argv); } -#else +#else boost::unit_test::test_suite* init_unit_test_suite(int argc, char* argv[]) { boost::unit_test::framework::master_test_suite().p_name.value = "TFileTransportTest"; @@ -414,4 +414,4 @@ boost::unit_test::test_suite* init_unit_test_suite(int argc, char* argv[]) { parse_args(argc, argv); return NULL; } -#endif \ No newline at end of file +#endif http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/test/concurrency/ThreadFactoryTests.h ---------------------------------------------------------------------- diff --git a/lib/cpp/test/concurrency/ThreadFactoryTests.h b/lib/cpp/test/concurrency/ThreadFactoryTests.h index 635c8a2..3ad14ca 100644 --- a/lib/cpp/test/concurrency/ThreadFactoryTests.h +++ b/lib/cpp/test/concurrency/ThreadFactoryTests.h @@ -102,7 +102,7 @@ public: PlatformThreadFactory threadFactory = PlatformThreadFactory(); - Monitor* monitor = new Monitor(); + shared_ptr<Monitor> monitor(new Monitor); for (int lix = 0; lix < loop; lix++) { http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/test/concurrency/ThreadManagerTests.h ---------------------------------------------------------------------- diff --git a/lib/cpp/test/concurrency/ThreadManagerTests.h b/lib/cpp/test/concurrency/ThreadManagerTests.h index 08e8179..b196813 100644 --- a/lib/cpp/test/concurrency/ThreadManagerTests.h +++ b/lib/cpp/test/concurrency/ThreadManagerTests.h @@ -45,7 +45,7 @@ public: public: Task(Monitor& monitor, size_t& count, int64_t timeout) - : _monitor(monitor), _count(count), _timeout(timeout), _done(false) {} + : _monitor(monitor), _count(count), _timeout(timeout), _startTime(0), _endTime(0), _done(false) {} void run() { http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/test/concurrency/TimerManagerTests.h ---------------------------------------------------------------------- diff --git a/lib/cpp/test/concurrency/TimerManagerTests.h b/lib/cpp/test/concurrency/TimerManagerTests.h index f4600fc..c6fa4cf 100644 --- a/lib/cpp/test/concurrency/TimerManagerTests.h +++ b/lib/cpp/test/concurrency/TimerManagerTests.h @@ -42,6 +42,7 @@ public: Task(Monitor& monitor, int64_t timeout) : _timeout(timeout), _startTime(Util::currentTime()), + _endTime(0), _monitor(monitor), _success(false), _done(false) {} http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/cpp/test/processor/EventLog.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/processor/EventLog.cpp b/lib/cpp/test/processor/EventLog.cpp index d4b8372..360307a 100644 --- a/lib/cpp/test/processor/EventLog.cpp +++ b/lib/cpp/test/processor/EventLog.cpp @@ -19,22 +19,26 @@ #include "EventLog.h" #include <stdarg.h> +#include <stdlib.h> using namespace std; using namespace apache::thrift::concurrency; namespace { -void debug(const char* fmt, ...) { - // Comment out this return to enable debug logs from the test code. - return; +// Define environment variable DEBUG_EVENTLOG to enable debug logging +// ex: $ DEBUG_EVENTLOG=1 processor_test +static const char * DEBUG_EVENTLOG = getenv("DEBUG_EVENTLOG"); - va_list ap; - va_start(ap, fmt); - vfprintf(stderr, fmt, ap); - va_end(ap); +void debug(const char* fmt, ...) { + if (DEBUG_EVENTLOG) { + va_list ap; + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); - fprintf(stderr, "\n"); + fprintf(stderr, "\n"); + } } } http://git-wip-us.apache.org/repos/asf/thrift/blob/36200904/lib/lua/src/usocket.c ---------------------------------------------------------------------- diff --git a/lib/lua/src/usocket.c b/lib/lua/src/usocket.c index be696e0..d97112c 100644 --- a/lib/lua/src/usocket.c +++ b/lib/lua/src/usocket.c @@ -58,13 +58,14 @@ T_ERRCODE socket_wait(p_socket sock, int mode, int timeout) { end = __gettime() + timeout/1000; do { + FD_ZERO(&rfds); + FD_ZERO(&wfds); + // Specify what I/O operations we care about if (mode & WAIT_MODE_R) { - FD_ZERO(&rfds); FD_SET(*sock, &rfds); } if (mode & WAIT_MODE_W) { - FD_ZERO(&wfds); FD_SET(*sock, &wfds); } @@ -131,8 +132,8 @@ T_ERRCODE socket_bind(p_socket sock, p_sa addr, int addr_len) { T_ERRCODE socket_get_info(p_socket sock, short *port, char *buf, size_t len) { struct sockaddr_in sa; - socklen_t addrlen; memset(&sa, 0, sizeof(sa)); + socklen_t addrlen = sizeof(sa); int rc = getsockname(*sock, (struct sockaddr*)&sa, &addrlen); if (!rc) { char *addr = inet_ntoa(sa.sin_addr);