Fix some implicit truncation and sign coersion in the networking APIs
authorChristopher Dykes <cdykes@fb.com>
Mon, 12 Dec 2016 19:34:23 +0000 (11:34 -0800)
committerFacebook Github Bot <facebook-github-bot-bot@fb.com>
Mon, 12 Dec 2016 19:48:05 +0000 (11:48 -0800)
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

19 files changed:
folly/IPAddress.cpp
folly/IPAddressV4.cpp
folly/IPAddressV4.h
folly/IPAddressV6.cpp
folly/IPAddressV6.h
folly/MacAddress.cpp
folly/SocketAddress.cpp
folly/detail/IPAddress.h
folly/detail/IPAddressSource.h
folly/io/async/AsyncSSLSocket.cpp
folly/io/async/AsyncServerSocket.cpp
folly/io/async/AsyncSignalHandler.cpp
folly/io/async/AsyncSocket.cpp
folly/io/async/AsyncSocket.h
folly/io/async/EventBase.cpp
folly/io/async/EventBase.h
folly/io/async/EventHandler.h
folly/io/async/HHWheelTimer.cpp
folly/io/async/NotificationQueue.h

index e1ac5b890d1b6f0765633911eff7c843f534d774..bb733743376b22a66c88bca7eb189521fa5b5d63 100644 (file)
@@ -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 {
index b8a0e0e2f847ad45935fac2b6881fd6db526c467..7a86b9a2201cc0c23f3843bb1f52f382c6267bf7 100644 (file)
@@ -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<std::string>("IPv4 addresses are 32 bits"));
index 7d3106685bfb11d7724fb36f3a1a393688cc439f..78d1368d40291f1dfe66e6debb8dcb644f3c5f0b 100644 (file)
@@ -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.
index 9904c406a5fc0f7d6142bda7a12fec87bfb12737..8860e92344d84082978870f6a42fae6a0268c358 100644 (file)
@@ -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.");
   }
index db70d09323d9e1d41535c466d37d9cf5a55080cc..eeccc89da61e7056330977d22537bfe52cea3aa4 100644 (file)
@@ -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.
index 1ce5d1ec3615ace3fd22a7798bcafc31545e864e..3b8dabaa104b77d435398cb1d8d0929b8fb07c5c 100644 (file)
@@ -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()) {
index 9e0eb0e4148ac47c513bff32fe3cf33a642b1a22..fe7b6f85b8939c2ec2cbacd0d8641e501104e2bd 100644 (file)
@@ -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);
   }
 }
 
index 3abaefc3a428e2383d66dc0d057eee19cc532ec9..c9f69ee996e15d192927efef43341dbbda46cc43 100644 (file)
@@ -43,7 +43,7 @@ inline std::string familyNameStr(sa_family_t family) {
 
 template <typename IPAddrType>
 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);
   }
index b4ea3dfdc581c39dd46b7eaca59921cb58bf6b8c..efa6eaea3b6eb1dfd9a9702c226b28cc5c0ac81d 100644 (file)
@@ -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<uint8_t, 3>(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));
 }
 }
 }
index 7a65d1a0c7a1b30e00a52cd7b5815be6a43c986e..8ce1644580d2ce2e3c0dfdc109c7219fff754609 100644 (file)
@@ -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(
index 3ad284c8b874c0a752e979fdec269193f3b21d15..cb4c1a2d2c42dc0feacbd13a4149eef74a66f4d0 100644 (file)
@@ -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 ",
index c8197cc8cddd725f2b68d097ab9d86564df6eb5e..31a61b28a9676610e569ab732e1e479e217e4da5 100644 (file)
@@ -84,7 +84,7 @@ void AsyncSignalHandler::libeventCallback(libevent_fd_t signum,
                                           short /* events */,
                                           void* arg) {
   AsyncSignalHandler* handler = static_cast<AsyncSignalHandler*>(arg);
-  handler->signalReceived(signum);
+  handler->signalReceived(int(signum));
 }
 
 } // folly
index 987dfa5af0ee52fdfcb6f848b0b70494eca7a70c..40cddc17044a84ce29f522f1c1e4b455bc566cd2 100644 (file)
@@ -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);
index fd2240f8e1ca0d86e0943e19639d55be3a9222df..973e493cb81f3adcaf1334654cb8d1e645088754 100644 (file)
@@ -635,7 +635,7 @@ class AsyncSocket : virtual public AsyncTransportWrapper {
     }
 
     void bytesWritten(size_t count) {
-      totalBytesWritten_ += count;
+      totalBytesWritten_ += uint32_t(count);
       socket_->appBytesWritten_ += count;
     }
 
index 4b3bfac29ab200d4b4bd6c7922ebaee8845d20f7..6ea6bdc94b2b71611cd3a01c8e55255fee5bef6e 100644 (file)
@@ -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) {
index 3320bf2651d062300fe7e88f4f7805e4693145af..9a5664bc559484ea20565202d82fea0d1fc4a1e8 100644 (file)
@@ -443,7 +443,7 @@ class EventBase : private boost::noncopyable,
    */
   void waitUntilRunning();
 
-  int getNotificationQueueSize() const;
+  size_t getNotificationQueueSize() const;
 
   void setMaxReadAtOnce(uint32_t maxAtOnce);
 
index b6a147cb2b8b410800c42b5f406673223a013520..9e46b9ac75e984777de3fdfcd174680e9474a345 100644 (file)
@@ -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;
   }
 
   /**
index 55bc89552a4b6c731188cd3f99802990191125ad..113fbac3e814118e65bbda2437cd5e8d74178af2 100644 (file)
@@ -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());
index 2baa2ba288f248b90e942fa1fb064d828c807543..88b1c7b9565874071e6360bc9c717d6f0a834aee 100644 (file)
@@ -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<ssize_t>(sizeof(signal));
+        bytes_expected = sizeof(signal);
         bytes_written = ::write(eventfd_, &signal, bytes_expected);
       } else {
         uint8_t signal = 1;
-        bytes_expected = static_cast<ssize_t>(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__