You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2017/10/29 11:11:47 UTC
thrift git commit: THRIFT-4376: fix more high impact coverity defects
Led to the discovery of incorrect lua socket error handling.
Repository: thrift
Updated Branches:
refs/heads/master 375bfee70 -> 533405e3f
THRIFT-4376: fix more high impact coverity defects
Led to the discovery of incorrect lua socket error handling.
This closes #1405
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/533405e3
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/533405e3
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/533405e3
Branch: refs/heads/master
Commit: 533405e3f85f2925aa1028fc3534e988e5debd32
Parents: 375bfee
Author: James E. King, III <jk...@apache.org>
Authored: Sat Oct 28 18:25:45 2017 -0400
Committer: James E. King, III <jk...@apache.org>
Committed: Sun Oct 29 07:05:47 2017 -0400
----------------------------------------------------------------------
.../cpp/src/thrift/generate/t_erl_generator.cc | 3 +
.../cpp/src/thrift/generate/t_java_generator.cc | 2 -
.../cpp/src/thrift/generate/t_st_generator.cc | 1 +
compiler/cpp/src/thrift/parse/t_const_value.h | 10 +--
compiler/cpp/src/thrift/platform.h | 7 +-
lib/cpp/src/thrift/transport/TSocket.cpp | 6 +-
lib/cpp/test/SecurityTest.cpp | 5 +-
lib/cpp/test/TBufferBaseTest.cpp | 4 +-
lib/cpp/test/TMemoryBufferTest.cpp | 11 +--
lib/cpp/test/TServerIntegrationTest.cpp | 1 +
.../apache/thrift/server/ServerTestBase.java | 74 ++++++++++----------
lib/lua/src/socket.h | 4 +-
lib/lua/src/usocket.c | 44 +++++++-----
lib/rs/src/server/threaded.rs | 2 +-
14 files changed, 92 insertions(+), 82 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/compiler/cpp/src/thrift/generate/t_erl_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_erl_generator.cc b/compiler/cpp/src/thrift/generate/t_erl_generator.cc
index ccd43b7..372c78b 100644
--- a/compiler/cpp/src/thrift/generate/t_erl_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_erl_generator.cc
@@ -57,6 +57,9 @@ public:
legacy_names_ = false;
maps_ = false;
otp16_ = false;
+ export_lines_first_ = true;
+ export_types_lines_first_ = true;
+
for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) {
if( iter->first.compare("legacynames") == 0) {
legacy_names_ = true;
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/compiler/cpp/src/thrift/generate/t_java_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc
index a5fb0e5..ebc8350 100644
--- a/compiler/cpp/src/thrift/generate/t_java_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc
@@ -2716,8 +2716,6 @@ std::string t_java_generator::get_java_type_string(t_type* type) {
} else {
throw std::runtime_error("Unknown thrift type \"" + type->get_name()
+ "\" passed to t_java_generator::get_java_type_string!");
- return "Unknown thrift type \"" + type->get_name()
- + "\" passed to t_java_generator::get_java_type_string!";
// This should never happen!
}
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/compiler/cpp/src/thrift/generate/t_st_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_st_generator.cc b/compiler/cpp/src/thrift/generate/t_st_generator.cc
index ffd7318..69ed776 100644
--- a/compiler/cpp/src/thrift/generate/t_st_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_st_generator.cc
@@ -56,6 +56,7 @@ public:
const std::string& option_string)
: t_oop_generator(program) {
(void)option_string;
+ temporary_var = 0;
std::map<std::string, std::string>::const_iterator iter;
/* no options yet */
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/compiler/cpp/src/thrift/parse/t_const_value.h
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/parse/t_const_value.h b/compiler/cpp/src/thrift/parse/t_const_value.h
index 5507803..fc2f648 100644
--- a/compiler/cpp/src/thrift/parse/t_const_value.h
+++ b/compiler/cpp/src/thrift/parse/t_const_value.h
@@ -38,13 +38,13 @@ void convert(From*, To&);
*/
class t_const_value {
public:
- enum t_const_value_type { CV_INTEGER, CV_DOUBLE, CV_STRING, CV_MAP, CV_LIST, CV_IDENTIFIER };
+ enum t_const_value_type { CV_INTEGER, CV_DOUBLE, CV_STRING, CV_MAP, CV_LIST, CV_IDENTIFIER, CV_UNKNOWN };
- t_const_value() {}
+ t_const_value() : intVal_(0), doubleVal_(0.0f), enum_((t_enum*)0), valType_(CV_UNKNOWN) {}
- t_const_value(int64_t val) { set_integer(val); }
+ t_const_value(int64_t val) : doubleVal_(0.0f), enum_((t_enum*)0), valType_(CV_UNKNOWN) { set_integer(val); }
- t_const_value(std::string val) { set_string(val); }
+ t_const_value(std::string val) : intVal_(0), doubleVal_(0.0f), enum_((t_enum*)0), valType_(CV_UNKNOWN) { set_string(val); }
void set_string(std::string val) {
valType_ = CV_STRING;
@@ -134,7 +134,7 @@ public:
void set_enum(t_enum* tenum) { enum_ = tenum; }
- t_const_value_type get_type() const { return valType_; }
+ t_const_value_type get_type() const { if (valType_ == CV_UNKNOWN) { throw std::string("unknown t_const_value"); } return valType_; }
private:
std::map<t_const_value*, t_const_value*> mapVal_;
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/compiler/cpp/src/thrift/platform.h
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/platform.h b/compiler/cpp/src/thrift/platform.h
index 7a8edae..f49adb9 100644
--- a/compiler/cpp/src/thrift/platform.h
+++ b/compiler/cpp/src/thrift/platform.h
@@ -34,10 +34,13 @@
#include <sys/stat.h>
#endif
+#include <cerrno>
+
+// ignore EEXIST, throw on any other error
#ifdef _WIN32
-#define MKDIR(x) mkdir(x)
+#define MKDIR(x) { int r = _mkdir(x); if (r == -1 && errno != EEXIST) { throw (std::string(x) + ": ") + strerror(errno); } }
#else
-#define MKDIR(x) mkdir(x, S_IRWXU | S_IRWXG | S_IRWXO)
+#define MKDIR(x) { int r = mkdir(x, S_IRWXU | S_IRWXG | S_IRWXO); if (r == -1 && errno != EEXIST) { throw (std::string(x) + ": ") + strerror(errno); } }
#endif
#ifdef PATH_MAX
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/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 @@ void TSocket::openConnection(struct addrinfo* res) {
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));
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/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 @@ BOOST_AUTO_TEST_CASE(ssl_security_matrix)
% 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],
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/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 @@ BOOST_AUTO_TEST_CASE( test_FramedTransport_Write_Read ) {
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_AUTO_TEST_CASE( test_FramedTransport_Write_Read ) {
}
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)));
}
}
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/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_AUTO_TEST_CASE(test_roundtrip) {
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);
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/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 @@ public:
*/
int getServerPort() {
TServerSocket* pSock = dynamic_cast<TServerSocket*>(pServer->getServerTransport().get());
+ if (!pSock) { throw std::logic_error("how come?"); }
return pSock->getPort();
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/lib/java/test/org/apache/thrift/server/ServerTestBase.java
----------------------------------------------------------------------
diff --git a/lib/java/test/org/apache/thrift/server/ServerTestBase.java b/lib/java/test/org/apache/thrift/server/ServerTestBase.java
index c2d2952..e3e4288 100644
--- a/lib/java/test/org/apache/thrift/server/ServerTestBase.java
+++ b/lib/java/test/org/apache/thrift/server/ServerTestBase.java
@@ -54,13 +54,13 @@ import thrift.test.Xtruct2;
public abstract class ServerTestBase extends TestCase {
public static class TestHandler implements ThriftTest.Iface {
-
+
public TestHandler() {}
-
+
public void testVoid() {
System.out.print("testVoid()\n");
}
-
+
public String testString(String thing) {
System.out.print("testString(\"" + thing + "\")\n");
return thing;
@@ -70,22 +70,22 @@ public abstract class ServerTestBase extends TestCase {
System.out.print("testBool(" + thing + ")\n");
return thing;
}
-
+
public byte testByte(byte thing) {
System.out.print("testByte(" + thing + ")\n");
return thing;
}
-
+
public int testI32(int thing) {
System.out.print("testI32(" + thing + ")\n");
return thing;
}
-
+
public long testI64(long thing) {
System.out.print("testI64(" + thing + ")\n");
return thing;
}
-
+
public double testDouble(double thing) {
System.out.print("testDouble(" + thing + ")\n");
return thing;
@@ -110,7 +110,7 @@ public abstract class ServerTestBase extends TestCase {
thing.i64_thing + "})\n");
return thing;
}
-
+
public Xtruct2 testNest(Xtruct2 nest) {
Xtruct thing = nest.struct_thing;
System.out.print("testNest({" +
@@ -122,7 +122,7 @@ public abstract class ServerTestBase extends TestCase {
nest.i32_thing + "})\n");
return nest;
}
-
+
public Map<Integer,Integer> testMap(Map<Integer,Integer> thing) {
System.out.print("testMap({");
System.out.print(thing);
@@ -136,7 +136,7 @@ public abstract class ServerTestBase extends TestCase {
System.out.print("})\n");
return thing;
}
-
+
public Set<Integer> testSet(Set<Integer> thing) {
System.out.print("testSet({");
boolean first = true;
@@ -151,7 +151,7 @@ public abstract class ServerTestBase extends TestCase {
System.out.print("})\n");
return thing;
}
-
+
public List<Integer> testList(List<Integer> thing) {
System.out.print("testList({");
boolean first = true;
@@ -166,58 +166,58 @@ public abstract class ServerTestBase extends TestCase {
System.out.print("})\n");
return thing;
}
-
+
public Numberz testEnum(Numberz thing) {
System.out.print("testEnum(" + thing + ")\n");
return thing;
}
-
+
public long testTypedef(long thing) {
System.out.print("testTypedef(" + thing + ")\n");
return thing;
}
-
+
public Map<Integer,Map<Integer,Integer>> testMapMap(int hello) {
System.out.print("testMapMap(" + hello + ")\n");
Map<Integer,Map<Integer,Integer>> mapmap =
new HashMap<Integer,Map<Integer,Integer>>();
-
+
HashMap<Integer,Integer> pos = new HashMap<Integer,Integer>();
HashMap<Integer,Integer> neg = new HashMap<Integer,Integer>();
for (int i = 1; i < 5; i++) {
pos.put(i, i);
neg.put(-i, -i);
}
-
+
mapmap.put(4, pos);
mapmap.put(-4, neg);
-
+
return mapmap;
}
-
+
public Map<Long, Map<Numberz,Insanity>> testInsanity(Insanity argument) {
System.out.print("testInsanity()\n");
-
+
HashMap<Numberz,Insanity> first_map = new HashMap<Numberz, Insanity>();
HashMap<Numberz,Insanity> second_map = new HashMap<Numberz, Insanity>();;
-
+
first_map.put(Numberz.TWO, argument);
first_map.put(Numberz.THREE, argument);
-
+
Insanity looney = new Insanity();
second_map.put(Numberz.SIX, looney);
-
+
Map<Long,Map<Numberz,Insanity>> insane =
new HashMap<Long, Map<Numberz,Insanity>>();
insane.put((long)1, first_map);
insane.put((long)2, second_map);
-
+
return insane;
}
-
+
public Xtruct testMulti(byte arg0, int arg1, long arg2, Map<Short,String> arg3, Numberz arg4, long arg5) {
System.out.print("testMulti()\n");
-
+
Xtruct hello = new Xtruct();;
hello.string_thing = "Hello2";
hello.byte_thing = arg0;
@@ -225,7 +225,7 @@ public abstract class ServerTestBase extends TestCase {
hello.i64_thing = arg2;
return hello;
}
-
+
public void testException(String arg) throws Xception, TException {
System.out.print("testException("+arg+")\n");
if ("Xception".equals(arg)) {
@@ -241,7 +241,7 @@ public abstract class ServerTestBase extends TestCase {
}
return;
}
-
+
public Xtruct testMultiException(String arg0, String arg1) throws Xception, Xception2 {
System.out.print("testMultiException(" + arg0 + ", " + arg1 + ")\n");
if (arg0.equals("Xception")) {
@@ -256,12 +256,12 @@ public abstract class ServerTestBase extends TestCase {
x.struct_thing.string_thing = "This is an Xception2";
throw x;
}
-
+
Xtruct result = new Xtruct();
result.string_thing = arg1;
return result;
}
-
+
public void testOneway(int sleepFor) {
System.out.println("testOneway(" + Integer.toString(sleepFor) +
") => sleeping...");
@@ -333,7 +333,7 @@ public abstract class ServerTestBase extends TestCase {
// todo: add assertions
private void testInsanity(ThriftTest.Client testClient) throws TException {
Insanity insane;
-
+
insane = new Insanity();
insane.userMap = new HashMap<Numberz, Long>();
insane.userMap.put(Numberz.FIVE, (long)5000);
@@ -351,7 +351,7 @@ public abstract class ServerTestBase extends TestCase {
for (long key : whoa.keySet()) {
Map<Numberz,Insanity> val = whoa.get(key);
System.out.print(key + " => {");
-
+
for (Numberz k2 : val.keySet()) {
Insanity v2 = val.get(k2);
System.out.print(k2 + " => {");
@@ -363,7 +363,7 @@ public abstract class ServerTestBase extends TestCase {
}
}
System.out.print("}, ");
-
+
List<Xtruct> xtructs = v2.xtructs;
System.out.print("{");
if (xtructs != null) {
@@ -372,7 +372,7 @@ public abstract class ServerTestBase extends TestCase {
}
}
System.out.print("}");
-
+
System.out.print("}, ");
}
System.out.print("}, ");
@@ -420,6 +420,7 @@ public abstract class ServerTestBase extends TestCase {
testOneway(testClient);
testI32(testClient);
transport.close();
+ socket.close();
stopServer();
}
@@ -430,7 +431,7 @@ public abstract class ServerTestBase extends TestCase {
}
public List<TProtocolFactory> getProtocols() {
- return PROTOCOLS;
+ return PROTOCOLS;
}
private void testList(ThriftTest.Client testClient) throws TException {
@@ -466,14 +467,14 @@ public abstract class ServerTestBase extends TestCase {
testClient.testMapMap(1);
Map<Integer,Map<Integer,Integer>> mapmap =
new HashMap<Integer,Map<Integer,Integer>>();
-
+
HashMap<Integer,Integer> pos = new HashMap<Integer,Integer>();
HashMap<Integer,Integer> neg = new HashMap<Integer,Integer>();
for (int i = 1; i < 5; i++) {
pos.put(i, i);
neg.put(-i, -i);
}
-
+
mapmap.put(4, pos);
mapmap.put(-4, neg);
assertEquals(mapmap, mm);
@@ -551,6 +552,7 @@ public abstract class ServerTestBase extends TestCase {
ThriftTest.Client testClient = new ThriftTest.Client(protocol);
assertEquals(0, testClient.testByte((byte) 0));
assertEquals(2, factory.count);
+ socket.close();
stopServer();
}
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/lib/lua/src/socket.h
----------------------------------------------------------------------
diff --git a/lib/lua/src/socket.h b/lib/lua/src/socket.h
index 8019ffe..afb827e 100644
--- a/lib/lua/src/socket.h
+++ b/lib/lua/src/socket.h
@@ -51,8 +51,8 @@ T_ERRCODE socket_send(p_socket sock, const char *data, size_t len, int timeout);
T_ERRCODE socket_recv(p_socket sock, char *data, size_t len, int timeout,
int *received);
-void socket_setblocking(p_socket sock);
-void socket_setnonblocking(p_socket sock);
+T_ERRCODE socket_setblocking(p_socket sock);
+T_ERRCODE socket_setnonblocking(p_socket sock);
T_ERRCODE socket_accept(p_socket sock, p_socket sibling,
p_sa addr, socklen_t *addr_len, int timeout);
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/lib/lua/src/usocket.c
----------------------------------------------------------------------
diff --git a/lib/lua/src/usocket.c b/lib/lua/src/usocket.c
index 864fa36..1a1b549 100644
--- a/lib/lua/src/usocket.c
+++ b/lib/lua/src/usocket.c
@@ -113,7 +113,7 @@ T_ERRCODE socket_create(p_socket sock, int domain, int type, int protocol) {
T_ERRCODE socket_destroy(p_socket sock) {
// TODO Figure out if I should be free-ing this
if (*sock > 0) {
- socket_setblocking(sock);
+ (void)socket_setblocking(sock);
close(*sock);
*sock = -1;
}
@@ -121,13 +121,15 @@ T_ERRCODE socket_destroy(p_socket sock) {
}
T_ERRCODE socket_bind(p_socket sock, p_sa addr, int addr_len) {
- int ret = SUCCESS;
- socket_setblocking(sock);
+ int ret = socket_setblocking(sock);
+ if (ret != SUCCESS) {
+ return ret;
+ }
if (bind(*sock, addr, addr_len)) {
ret = errno;
}
- socket_setnonblocking(sock);
- return ret;
+ int ret2 = socket_setnonblocking(sock);
+ return ret == SUCCESS ? ret2 : ret;
}
T_ERRCODE socket_get_info(p_socket sock, short *port, char *buf, size_t len) {
@@ -168,22 +170,25 @@ T_ERRCODE socket_accept(p_socket sock, p_socket client,
if (*client > 0) {
return SUCCESS;
}
- err = errno;
- } while (err != EINTR);
+ } while ((err = errno) == EINTR);
+
if (err == EAGAIN || err == ECONNABORTED) {
return socket_wait(sock, WAIT_MODE_R, timeout);
}
+
return err;
}
T_ERRCODE socket_listen(p_socket sock, int backlog) {
- int ret = SUCCESS;
- socket_setblocking(sock);
+ int ret = socket_setblocking(sock);
+ if (ret != SUCCESS) {
+ return ret;
+ }
if (listen(*sock, backlog)) {
ret = errno;
}
- socket_setnonblocking(sock);
- return ret;
+ int ret2 = socket_setnonblocking(sock);
+ return ret == SUCCESS ? ret2 : ret;
}
////////////////////////////////////////////////////////////////////////////////
@@ -217,12 +222,12 @@ T_ERRCODE socket_send(
if (put > 0) {
return SUCCESS;
}
- err = errno;
- } while (err != EINTR);
+ } while ((err = errno) == EINTR);
if (err == EAGAIN) {
return socket_wait(sock, WAIT_MODE_W, timeout);
}
+
return err;
}
@@ -232,8 +237,8 @@ T_ERRCODE socket_recv(
if (*sock < 0) {
return CLOSED;
}
+ *received = 0;
- int flags = fcntl(*sock, F_GETFL, 0);
do {
got = recv(*sock, data, len, 0);
if (got > 0) {
@@ -246,27 +251,28 @@ T_ERRCODE socket_recv(
if (got == 0) {
return CLOSED;
}
- } while (err != EINTR);
+ } while (err == EINTR);
if (err == EAGAIN) {
return socket_wait(sock, WAIT_MODE_R, timeout);
}
+
return err;
}
////////////////////////////////////////////////////////////////////////////////
// Util
-void socket_setnonblocking(p_socket sock) {
+T_ERRCODE socket_setnonblocking(p_socket sock) {
int flags = fcntl(*sock, F_GETFL, 0);
flags |= O_NONBLOCK;
- fcntl(*sock, F_SETFL, flags);
+ return fcntl(*sock, F_SETFL, flags) != -1 ? SUCCESS : errno;
}
-void socket_setblocking(p_socket sock) {
+T_ERRCODE socket_setblocking(p_socket sock) {
int flags = fcntl(*sock, F_GETFL, 0);
flags &= (~(O_NONBLOCK));
- fcntl(*sock, F_SETFL, flags);
+ return fcntl(*sock, F_SETFL, flags) != -1 ? SUCCESS : errno;
}
////////////////////////////////////////////////////////////////////////////////
http://git-wip-us.apache.org/repos/asf/thrift/blob/533405e3/lib/rs/src/server/threaded.rs
----------------------------------------------------------------------
diff --git a/lib/rs/src/server/threaded.rs b/lib/rs/src/server/threaded.rs
index 66680b1..515b20d 100644
--- a/lib/rs/src/server/threaded.rs
+++ b/lib/rs/src/server/threaded.rs
@@ -155,7 +155,7 @@ impl<PRC, RTF, IPF, WTF, OPF> TServer<PRC, RTF, IPF, WTF, OPF>
w_trans_factory: write_transport_factory,
o_proto_factory: output_protocol_factory,
processor: Arc::new(processor),
- worker_pool: ThreadPool::new_with_name(
+ worker_pool: ThreadPool::with_name(
"Thrift service processor".to_owned(),
num_workers,
),