From: Christopher Dykes Date: Mon, 12 Dec 2016 19:34:23 +0000 (-0800) Subject: Fix some implicit truncation and sign coersion in the networking APIs X-Git-Tag: v2016.12.19.00~32 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=b4a27a035cc092f444df9a952e371ed4a201c2f4;p=folly.git Fix some implicit truncation and sign coersion in the networking APIs Summary: Split out of an earlier pair of diffs, this focuses exclusively on Folly's networking APIs and works to eliminate places where values were being implicitly truncated. Reviewed By: yfeldblum Differential Revision: D4288042 fbshipit-source-id: dd6e19acd319296a45c29c8050dc62f06571e1e6 --- diff --git a/folly/IPAddress.cpp b/folly/IPAddress.cpp index e1ac5b89..bb733743 100644 --- a/folly/IPAddress.cpp +++ b/folly/IPAddress.cpp @@ -89,7 +89,8 @@ CIDRNetwork IPAddress::createNetwork(StringPiece ipSlashCidr, "'")); } IPAddress subnet(vec.at(0)); - uint8_t cidr = (defaultCidr > -1) ? defaultCidr : (subnet.isV4() ? 32 : 128); + uint8_t cidr = + (defaultCidr > -1) ? uint8_t(defaultCidr) : (subnet.isV4() ? 32 : 128); if (elemCount == 2) { try { diff --git a/folly/IPAddressV4.cpp b/folly/IPAddressV4.cpp index b8a0e0e2..7a86b9a2 100644 --- a/folly/IPAddressV4.cpp +++ b/folly/IPAddressV4.cpp @@ -236,7 +236,7 @@ uint8_t IPAddressV4::getNthMSByte(size_t byteIndex) const { } // protected const ByteArray4 IPAddressV4::fetchMask(size_t numBits) { - static const uint8_t bits = bitCount(); + static const size_t bits = bitCount(); if (numBits > bits) { throw IPAddressFormatException( to("IPv4 addresses are 32 bits")); diff --git a/folly/IPAddressV4.h b/folly/IPAddressV4.h index 7d310668..78d1368d 100644 --- a/folly/IPAddressV4.h +++ b/folly/IPAddressV4.h @@ -204,7 +204,7 @@ class IPAddressV4 { std::string toFullyQualified() const { return str(); } // @see IPAddress#version - size_t version() const { return 4; } + uint8_t version() const { return 4; } /** * Return the mask associated with the given number of bits. diff --git a/folly/IPAddressV6.cpp b/folly/IPAddressV6.cpp index 9904c406..8860e923 100644 --- a/folly/IPAddressV6.cpp +++ b/folly/IPAddressV6.cpp @@ -89,7 +89,7 @@ IPAddressV6::IPAddressV6(StringPiece addr) { if (!getaddrinfo(ip.c_str(), nullptr, &hints, &result)) { struct sockaddr_in6* ipAddr = (struct sockaddr_in6*)result->ai_addr; addr_.in6Addr_ = ipAddr->sin6_addr; - scope_ = ipAddr->sin6_scope_id; + scope_ = uint16_t(ipAddr->sin6_scope_id); freeaddrinfo(result); } else { throw IPAddressFormatException( @@ -106,7 +106,7 @@ IPAddressV6::IPAddressV6(const in6_addr& src) // sockaddr_in6 constructor IPAddressV6::IPAddressV6(const sockaddr_in6& src) : addr_(src.sin6_addr) - , scope_(src.sin6_scope_id) + , scope_(uint16_t(src.sin6_scope_id)) { } @@ -384,7 +384,7 @@ uint8_t IPAddressV6::getNthMSByte(size_t byteIndex) const { // protected const ByteArray16 IPAddressV6::fetchMask(size_t numBits) { - static const uint8_t bits = bitCount(); + static const size_t bits = bitCount(); if (numBits > bits) { throw IPAddressFormatException("IPv6 addresses are 128 bits."); } diff --git a/folly/IPAddressV6.h b/folly/IPAddressV6.h index db70d093..eeccc89d 100644 --- a/folly/IPAddressV6.h +++ b/folly/IPAddressV6.h @@ -267,7 +267,7 @@ class IPAddressV6 { std::string str() const; // @see IPAddress#version - size_t version() const { return 6; } + uint8_t version() const { return 6; } /** * Return the solicited-node multicast address for this address. diff --git a/folly/MacAddress.cpp b/folly/MacAddress.cpp index 1ce5d1ec..3b8dabaa 100644 --- a/folly/MacAddress.cpp +++ b/folly/MacAddress.cpp @@ -128,7 +128,7 @@ void MacAddress::parse(StringPiece str) { } // Update parsed with the newly parsed byte - parsed[byteIndex] = ((upper << 4) | lower); + parsed[byteIndex] = uint8_t((upper << 4) | lower); } if (p != str.end()) { diff --git a/folly/SocketAddress.cpp b/folly/SocketAddress.cpp index 9e0eb0e4..fe7b6f85 100644 --- a/folly/SocketAddress.cpp +++ b/folly/SocketAddress.cpp @@ -229,7 +229,7 @@ void SocketAddress::setFromPath(StringPiece path) { } size_t len = path.size(); - storage_.un.len = offsetof(struct sockaddr_un, sun_path) + len; + storage_.un.len = socklen_t(offsetof(struct sockaddr_un, sun_path) + len); memcpy(storage_.un.addr->sun_path, path.data(), len); // If there is room, put a terminating NUL byte in sun_path. In general the // path should be NUL terminated, although getsockname() and getpeername() @@ -631,7 +631,7 @@ struct addrinfo* SocketAddress::getAddrInfo(const char* host, } void SocketAddress::setFromAddrInfo(const struct addrinfo* info) { - setFromSockaddr(info->ai_addr, info->ai_addrlen); + setFromSockaddr(info->ai_addr, socklen_t(info->ai_addrlen)); } void SocketAddress::setFromLocalAddr(const struct addrinfo* info) { @@ -639,13 +639,13 @@ void SocketAddress::setFromLocalAddr(const struct addrinfo* info) { // can be mapped into IPv6 space. for (const struct addrinfo* ai = info; ai != nullptr; ai = ai->ai_next) { if (ai->ai_family == AF_INET6) { - setFromSockaddr(ai->ai_addr, ai->ai_addrlen); + setFromSockaddr(ai->ai_addr, socklen_t(ai->ai_addrlen)); return; } } // Otherwise, just use the first address in the list. - setFromSockaddr(info->ai_addr, info->ai_addrlen); + setFromSockaddr(info->ai_addr, socklen_t(info->ai_addrlen)); } void SocketAddress::setFromSocket( @@ -707,7 +707,8 @@ void SocketAddress::updateUnixAddressLength(socklen_t addrlen) { // Call strnlen(), just in case the length was overspecified. socklen_t maxLength = addrlen - offsetof(struct sockaddr_un, sun_path); size_t pathLength = strnlen(storage_.un.addr->sun_path, maxLength); - storage_.un.len = offsetof(struct sockaddr_un, sun_path) + pathLength; + storage_.un.len = + socklen_t(offsetof(struct sockaddr_un, sun_path) + pathLength); } } diff --git a/folly/detail/IPAddress.h b/folly/detail/IPAddress.h index 3abaefc3..c9f69ee9 100644 --- a/folly/detail/IPAddress.h +++ b/folly/detail/IPAddress.h @@ -43,7 +43,7 @@ inline std::string familyNameStr(sa_family_t family) { template inline bool -getNthMSBitImpl(const IPAddrType& ip, uint8_t bitIndex, sa_family_t family) { +getNthMSBitImpl(const IPAddrType& ip, size_t bitIndex, sa_family_t family) { if (bitIndex >= ip.bitCount()) { getNthMSBitImplThrow(ip.bitCount(), family); } diff --git a/folly/detail/IPAddressSource.h b/folly/detail/IPAddressSource.h index b4ea3dfd..efa6eaea 100644 --- a/folly/detail/IPAddressSource.h +++ b/folly/detail/IPAddressSource.h @@ -203,7 +203,7 @@ inline void writeIntegerString(IntegralType val, char** buffer) { } else { value += ('a' - 10); } - *(buf++) = value; + *(buf++) = char(value); val %= powerToPrint; found = true; } @@ -227,7 +227,7 @@ inline std::string fastIpv4ToString(const in_addr& inAddr) { *(buf++) = '.'; writeIntegerString(octets[3], &buf); - return std::string(str, buf - str); + return std::string(str, size_t(buf - str)); } inline std::string fastIpv6ToString(const in6_addr& in6Addr) { @@ -251,7 +251,7 @@ inline std::string fastIpv6ToString(const in6_addr& in6Addr) { } } - return std::string(str, buf - str); + return std::string(str, size_t(buf - str)); } } } diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp index 7a65d1a0..8ce16445 100644 --- a/folly/io/async/AsyncSSLSocket.cpp +++ b/folly/io/async/AsyncSSLSocket.cpp @@ -730,7 +730,8 @@ void AsyncSSLSocket::startSSLConnect() { // Make end time at least >= start time. handshakeEndTime_ = handshakeStartTime_; if (handshakeConnectTimeout_ > 0) { - handshakeTimeout_.scheduleTimeout(handshakeConnectTimeout_); + handshakeTimeout_.scheduleTimeout( + std::chrono::milliseconds(handshakeConnectTimeout_)); } handleConnect(); } @@ -1159,9 +1160,8 @@ void AsyncSSLSocket::scheduleConnectTimeout() { assert(connectCallback_ == nullptr); // We use a different connect timeout here than the handshake timeout, so // that we can disambiguate the 2 timers. - int timeout = connectTimeout_.count(); - if (timeout > 0) { - if (!connectionTimeout_.scheduleTimeout(timeout)) { + if (connectTimeout_.count() > 0) { + if (!connectionTimeout_.scheduleTimeout(connectTimeout_)) { throw AsyncSocketException( AsyncSocketException::INTERNAL_ERROR, withAddr("failed to schedule AsyncSSLSocket connect timeout")); @@ -1234,9 +1234,9 @@ AsyncSSLSocket::performRead(void** buf, size_t* buflen, size_t* offset) { return AsyncSocket::performRead(buf, buflen, offset); } - ssize_t bytes = 0; + int bytes = 0; if (!isBufferMovable_) { - bytes = SSL_read(ssl_, *buf, *buflen); + bytes = SSL_read(ssl_, *buf, int(*buflen)); } #ifdef SSL_MODE_MOVE_BUFFER_OWNERSHIP else { @@ -1451,17 +1451,17 @@ AsyncSocket::WriteResult AsyncSSLSocket::performWrite( bytes = eorAwareSSLWrite( ssl_, sslWriteBuf, - len, + int(len), (isSet(flags, WriteFlags::EOR) && i + buffersStolen + 1 == count)); if (bytes <= 0) { - int error = SSL_get_error(ssl_, bytes); + int error = SSL_get_error(ssl_, int(bytes)); if (error == SSL_ERROR_WANT_WRITE) { // The caller will register for write event if not already. - *partialWritten = offset; + *partialWritten = uint32_t(offset); return WriteResult(totalWritten); } - auto writeResult = interpretSSLError(bytes, error); + auto writeResult = interpretSSLError(int(bytes), error); if (writeResult.writeReturn < 0) { return writeResult; } // else fall through to below to correctly record totalWritten @@ -1484,7 +1484,7 @@ AsyncSocket::WriteResult AsyncSSLSocket::performWrite( (*countWritten)++; v = &(vec[++i]); } - *partialWritten = bytes; + *partialWritten = uint32_t(bytes); return WriteResult(totalWritten); } } @@ -1576,11 +1576,11 @@ int AsyncSSLSocket::bioWrite(BIO* b, const char* in, int inl) { OpenSSLUtils::getBioFd(b, nullptr), &msg, flags); BIO_clear_retry_flags(b); if (!result.exception && result.writeReturn <= 0) { - if (OpenSSLUtils::getBioShouldRetryWrite(result.writeReturn)) { + if (OpenSSLUtils::getBioShouldRetryWrite(int(result.writeReturn))) { BIO_set_retry_write(b); } } - return result.writeReturn; + return int(result.writeReturn); } int AsyncSSLSocket::sslVerifyCallback( diff --git a/folly/io/async/AsyncServerSocket.cpp b/folly/io/async/AsyncServerSocket.cpp index 3ad284c8..cb4c1a2d 100644 --- a/folly/io/async/AsyncServerSocket.cpp +++ b/folly/io/async/AsyncServerSocket.cpp @@ -404,7 +404,7 @@ void AsyncServerSocket::bind(uint16_t port) { } // Bind to the socket - if (fsp::bind(s, res->ai_addr, res->ai_addrlen) != 0) { + if (fsp::bind(s, res->ai_addr, socklen_t(res->ai_addrlen)) != 0) { folly::throwSystemError( errno, "failed to bind to async server socket for port ", diff --git a/folly/io/async/AsyncSignalHandler.cpp b/folly/io/async/AsyncSignalHandler.cpp index c8197cc8..31a61b28 100644 --- a/folly/io/async/AsyncSignalHandler.cpp +++ b/folly/io/async/AsyncSignalHandler.cpp @@ -84,7 +84,7 @@ void AsyncSignalHandler::libeventCallback(libevent_fd_t signum, short /* events */, void* arg) { AsyncSignalHandler* handler = static_cast(arg); - handler->signalReceived(signum); + handler->signalReceived(int(signum)); } } // folly diff --git a/folly/io/async/AsyncSocket.cpp b/folly/io/async/AsyncSocket.cpp index 987dfa5a..40cddc17 100644 --- a/folly/io/async/AsyncSocket.cpp +++ b/folly/io/async/AsyncSocket.cpp @@ -489,10 +489,10 @@ int AsyncSocket::socketConnect(const struct sockaddr* saddr, socklen_t len) { void AsyncSocket::scheduleConnectTimeout() { // Connection in progress. - int timeout = connectTimeout_.count(); + auto timeout = connectTimeout_.count(); if (timeout > 0) { // Start a timer in case the connection takes too long. - if (!writeTimeout_.scheduleTimeout(timeout)) { + if (!writeTimeout_.scheduleTimeout(uint32_t(timeout))) { throw AsyncSocketException( AsyncSocketException::INTERNAL_ERROR, withAddr("failed to schedule AsyncSocket connect timeout")); @@ -715,7 +715,7 @@ void AsyncSocket::writeImpl(WriteCallback* callback, const iovec* vec, uint32_t countWritten = 0; uint32_t partialWritten = 0; - int bytesWritten = 0; + ssize_t bytesWritten = 0; bool mustRegister = false; if ((state_ == StateEnum::ESTABLISHED || state_ == StateEnum::FAST_OPEN) && !connecting()) { @@ -725,8 +725,8 @@ void AsyncSocket::writeImpl(WriteCallback* callback, const iovec* vec, assert(writeReqTail_ == nullptr); assert((eventFlags_ & EventHandler::WRITE) == 0); - auto writeResult = - performWrite(vec, count, flags, &countWritten, &partialWritten); + auto writeResult = performWrite( + vec, uint32_t(count), flags, &countWritten, &partialWritten); bytesWritten = writeResult.writeReturn; if (bytesWritten < 0) { auto errnoCopy = errno; @@ -766,14 +766,20 @@ void AsyncSocket::writeImpl(WriteCallback* callback, const iovec* vec, // Create a new WriteRequest to add to the queue WriteRequest* req; try { - req = BytesWriteRequest::newRequest(this, callback, vec + countWritten, - count - countWritten, partialWritten, - bytesWritten, std::move(ioBuf), flags); + req = BytesWriteRequest::newRequest( + this, + callback, + vec + countWritten, + uint32_t(count - countWritten), + partialWritten, + uint32_t(bytesWritten), + std::move(ioBuf), + flags); } catch (const std::exception& ex) { // we mainly expect to catch std::bad_alloc here AsyncSocketException tex(AsyncSocketException::INTERNAL_ERROR, withAddr(string("failed to append new WriteRequest: ") + ex.what())); - return failWrite(__func__, callback, bytesWritten, tex); + return failWrite(__func__, callback, size_t(bytesWritten), tex); } req->consume(); if (writeReqTail_ == nullptr) { @@ -1187,8 +1193,12 @@ int AsyncSocket::setCongestionFlavor(const std::string &cname) { } - if (setsockopt(fd_, IPPROTO_TCP, TCP_CONGESTION, cname.c_str(), - cname.length() + 1) != 0) { + if (setsockopt( + fd_, + IPPROTO_TCP, + TCP_CONGESTION, + cname.c_str(), + socklen_t(cname.length() + 1)) != 0) { int errnoCopy = errno; VLOG(2) << "failed to update TCP_CONGESTION option on AsyncSocket " << this << "(fd=" << fd_ << ", state=" << state_ << "): " @@ -1897,7 +1907,7 @@ AsyncSocket::WriteResult AsyncSocket::performWrite( uint32_t bytesWritten; uint32_t n; - for (bytesWritten = totalWritten, n = 0; n < count; ++n) { + for (bytesWritten = uint32_t(totalWritten), n = 0; n < count; ++n) { const iovec* v = vec + n; if (v->iov_len > bytesWritten) { // Partial write finished in the middle of this iovec @@ -1906,7 +1916,7 @@ AsyncSocket::WriteResult AsyncSocket::performWrite( return WriteResult(totalWritten); } - bytesWritten -= v->iov_len; + bytesWritten -= uint32_t(v->iov_len); } assert(bytesWritten == 0); diff --git a/folly/io/async/AsyncSocket.h b/folly/io/async/AsyncSocket.h index fd2240f8..973e493c 100644 --- a/folly/io/async/AsyncSocket.h +++ b/folly/io/async/AsyncSocket.h @@ -635,7 +635,7 @@ class AsyncSocket : virtual public AsyncTransportWrapper { } void bytesWritten(size_t count) { - totalBytesWritten_ += count; + totalBytesWritten_ += uint32_t(count); socket_->appBytesWritten_ += count; } diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp index 4b3bfac2..6ea6bdc9 100644 --- a/folly/io/async/EventBase.cpp +++ b/folly/io/async/EventBase.cpp @@ -112,7 +112,7 @@ EventBase::EventBase(bool enableTimeMeasurement) , avgLoopTime_(2000000) , maxLatencyLoopTime_(avgLoopTime_) , enableTimeMeasurement_(enableTimeMeasurement) - , nextLoopCnt_(-40) // Early wrap-around so bugs will manifest soon + , nextLoopCnt_(uint64_t(-40)) // Early wrap-around so bugs will manifest soon , latestLoopCnt_(nextLoopCnt_) , startWork_(0) , observer_(nullptr) @@ -158,7 +158,7 @@ EventBase::EventBase(event_base* evb, bool enableTimeMeasurement) , avgLoopTime_(2000000) , maxLatencyLoopTime_(avgLoopTime_) , enableTimeMeasurement_(enableTimeMeasurement) - , nextLoopCnt_(-40) // Early wrap-around so bugs will manifest soon + , nextLoopCnt_(uint64_t(-40)) // Early wrap-around so bugs will manifest soon , latestLoopCnt_(nextLoopCnt_) , startWork_(0) , observer_(nullptr) @@ -216,7 +216,7 @@ EventBase::~EventBase() { VLOG(5) << "EventBase(): Destroyed."; } -int EventBase::getNotificationQueueSize() const { +size_t EventBase::getNotificationQueueSize() const { return queue_->size(); } @@ -717,8 +717,8 @@ bool EventBase::scheduleTimeout(AsyncTimeout* obj, assert(isInEventBaseThread()); // Set up the timeval and add the event struct timeval tv; - tv.tv_sec = timeout.count() / 1000LL; - tv.tv_usec = (timeout.count() % 1000LL) * 1000LL; + tv.tv_sec = long(timeout.count() / 1000LL); + tv.tv_usec = long((timeout.count() % 1000LL) * 1000LL); struct event* ev = obj->getEvent(); if (event_add(ev, &tv) < 0) { diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h index 3320bf26..9a5664bc 100644 --- a/folly/io/async/EventBase.h +++ b/folly/io/async/EventBase.h @@ -443,7 +443,7 @@ class EventBase : private boost::noncopyable, */ void waitUntilRunning(); - int getNotificationQueueSize() const; + size_t getNotificationQueueSize() const; void setMaxReadAtOnce(uint32_t maxAtOnce); diff --git a/folly/io/async/EventHandler.h b/folly/io/async/EventHandler.h index b6a147cb..9e46b9ac 100644 --- a/folly/io/async/EventHandler.h +++ b/folly/io/async/EventHandler.h @@ -152,8 +152,7 @@ class EventHandler : private boost::noncopyable { * Return the set of events that we're currently registered for. */ uint16_t getRegisteredEvents() const { - return (isHandlerRegistered()) ? - event_.ev_events : 0; + return (isHandlerRegistered()) ? uint16_t(event_.ev_events) : 0u; } /** diff --git a/folly/io/async/HHWheelTimer.cpp b/folly/io/async/HHWheelTimer.cpp index 55bc8955..113fbac3 100644 --- a/folly/io/async/HHWheelTimer.cpp +++ b/folly/io/async/HHWheelTimer.cpp @@ -281,7 +281,7 @@ size_t HHWheelTimer::cancelAll() { void HHWheelTimer::scheduleNextTimeout() { auto nextTick = calcNextTick(); - long tick = 1; + int64_t tick = 1; if (nextTick & WHEEL_MASK) { auto bi = makeBitIterator(bitmap_.begin()); diff --git a/folly/io/async/NotificationQueue.h b/folly/io/async/NotificationQueue.h index 2baa2ba2..88b1c7b9 100644 --- a/folly/io/async/NotificationQueue.h +++ b/folly/io/async/NotificationQueue.h @@ -488,17 +488,17 @@ class NotificationQueue { } ssize_t bytes_written = 0; - ssize_t bytes_expected = 0; + size_t bytes_expected = 0; do { if (eventfd_ >= 0) { // eventfd(2) dictates that we must write a 64-bit integer uint64_t signal = 1; - bytes_expected = static_cast(sizeof(signal)); + bytes_expected = sizeof(signal); bytes_written = ::write(eventfd_, &signal, bytes_expected); } else { uint8_t signal = 1; - bytes_expected = static_cast(sizeof(signal)); + bytes_expected = sizeof(signal); bytes_written = ::write(pipeFds_[1], &signal, bytes_expected); } } while (bytes_written == -1 && errno == EINTR); @@ -510,7 +510,7 @@ class NotificationQueue { } #endif - if (bytes_written == bytes_expected) { + if (bytes_written == ssize_t(bytes_expected)) { signal_ = true; } else { #ifdef __ANDROID__