You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2016/10/13 21:00:07 UTC
thrift git commit: THRIFT-3943: resolve some high severity
outstanding defects identified by coverity scan Clients: C++,
Lua Patch: James E. King, III
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 <ji...@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 <ji...@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);