commit 533405e3f85f2925aa1028fc3534e988e5debd32
author James E. King, III <jking@apache.org> Sat Oct 28 18:25:45 2017 -0400
committer James E. King, III <jking@apache.org> Sun Oct 29 07:05:47 2017 -0400
tree 58073ef89cddef4bcae5e9b731ea028bdf105948
parent 375bfee701d3caa74074f8afb3d6940e52c45c88

THRIFT-4376: fix more high impact coverity defects
Led to the discovery of incorrect lua socket error handling.

This closes #1405

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 7f8d7af..d93d0ff 100644
--- a/lib/cpp/src/thrift/transport/TSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSocket.cpp
@@ -385,7 +385,11 @@
 
 done:
   // Set socket back to normal mode (blocking)
-  THRIFT_FCNTL(socket_, THRIFT_F_SETFL, flags);
+  if (-1 == THRIFT_FCNTL(socket_, THRIFT_F_SETFL, flags)) {
+    int errno_copy = THRIFT_GET_SOCKET_ERROR;
+    GlobalOutput.perror("TSocket::open() THRIFT_FCNTL " + getSocketInfo(), errno_copy);
+    throw TTransportException(TTransportException::NOT_OPEN, "THRIFT_FCNTL() failed", errno_copy);
+  }
 
   if (path_.empty()) {
     setCachedAddress(res->ai_addr, static_cast<socklen_t>(res->ai_addrlen));

lib/cpp/test/SecurityTest.cpp

diff --git a/lib/cpp/test/SecurityTest.cpp b/lib/cpp/test/SecurityTest.cpp
index 6eb1fe3..51ee427 100644
--- a/lib/cpp/test/SecurityTest.cpp
+++ b/lib/cpp/test/SecurityTest.cpp
@@ -255,11 +255,12 @@
                     % protocol2str(si) % protocol2str(ci));
 
                 mConnected = false;
+                // thread_group manages the thread lifetime - ignore the return value of create_thread
                 boost::thread_group threads;
-                threads.create_thread(bind(&SecurityFixture::server, this, static_cast<apache::thrift::transport::SSLProtocol>(si)));
+                (void)threads.create_thread(bind(&SecurityFixture::server, this, static_cast<apache::thrift::transport::SSLProtocol>(si)));
                 mCVar.wait(lock);           // wait for listen() to succeed
                 lock.unlock();
-                threads.create_thread(bind(&SecurityFixture::client, this, static_cast<apache::thrift::transport::SSLProtocol>(ci)));
+                (void)threads.create_thread(bind(&SecurityFixture::client, this, static_cast<apache::thrift::transport::SSLProtocol>(ci)));
                 threads.join_all();
 
                 BOOST_CHECK_MESSAGE(mConnected == matrix[ci][si],

lib/cpp/test/TBufferBaseTest.cpp

diff --git a/lib/cpp/test/TBufferBaseTest.cpp b/lib/cpp/test/TBufferBaseTest.cpp
index 4e3509e..4201ddb 100644
--- a/lib/cpp/test/TBufferBaseTest.cpp
+++ b/lib/cpp/test/TBufferBaseTest.cpp
@@ -567,7 +567,7 @@
       for (int d1 = 0; d1 < 3; d1++) {
         shared_ptr<TMemoryBuffer> buffer(new TMemoryBuffer(16));
         TFramedTransport trans(buffer, size);
-        uint8_t data_out[1<<15];
+        std::vector<uint8_t> data_out(1<<17, 0);
         std::vector<int> flush_sizes;
 
         int write_offset = 0;
@@ -605,7 +605,7 @@
         }
 
         BOOST_CHECK_EQUAL((unsigned int)read_offset, sizeof(data));
-        BOOST_CHECK(!memcmp(data, data_out, sizeof(data)));
+        BOOST_CHECK(!memcmp(data, &data_out[0], sizeof(data)));
       }
     }
   }

lib/cpp/test/TMemoryBufferTest.cpp

diff --git a/lib/cpp/test/TMemoryBufferTest.cpp b/lib/cpp/test/TMemoryBufferTest.cpp
index 4384187..9492f69 100644
--- a/lib/cpp/test/TMemoryBufferTest.cpp
+++ b/lib/cpp/test/TMemoryBufferTest.cpp
@@ -80,20 +80,11 @@
   BOOST_CHECK(a == a2);
 }
 
-BOOST_AUTO_TEST_CASE(test_copy) {
+BOOST_AUTO_TEST_CASE(test_readAppendToString) {
   string* str1 = new string("abcd1234");
-  ptrdiff_t str1addr = reinterpret_cast<ptrdiff_t>(str1);
-  const char* data1 = str1->data();
   TMemoryBuffer buf((uint8_t*)str1->data(),
                     static_cast<uint32_t>(str1->length()),
                     TMemoryBuffer::COPY);
-  delete str1;
-  string* str2 = new string("plsreuse");
-  bool obj_reuse = (str1addr == reinterpret_cast<ptrdiff_t>(str2));
-  bool dat_reuse = (data1 == str2->data());
-  BOOST_TEST_MESSAGE("Object reuse: " << obj_reuse << "   Data reuse: " << dat_reuse
-                << ((obj_reuse && dat_reuse) ? "   YAY!" : ""));
-  delete str2;
 
   string str3 = "wxyz", str4 = "6789";
   buf.readAppendToString(str3, 4);

lib/cpp/test/TServerIntegrationTest.cpp

diff --git a/lib/cpp/test/TServerIntegrationTest.cpp b/lib/cpp/test/TServerIntegrationTest.cpp
index 12657d4..5f7b2e8 100644
--- a/lib/cpp/test/TServerIntegrationTest.cpp
+++ b/lib/cpp/test/TServerIntegrationTest.cpp
@@ -265,6 +265,7 @@
    */
   int getServerPort() {
     TServerSocket* pSock = dynamic_cast<TServerSocket*>(pServer->getServerTransport().get());
+    if (!pSock) { throw std::logic_error("how come?"); }
     return pSock->getPort();
   }