You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2017/02/17 01:50:54 UTC
[2/7] mesos git commit: Stout: Dealt with some MSVC compiler warnings.
Stout: Dealt with some MSVC compiler warnings.
The warnings dealt with in this commit are implicit type conversions,
mostly to/from integers.
Two notable tweaks are:
* `struct timeval` construction in `stout/duration.hpp` no longer
performs an `int64_t -> double -> long` chain of casts.
* Protobuf (de)serialization will now CHECK if the input string
can be safely casted to a signed integer.
Review: https://reviews.apache.org/r/56591
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e4d8573d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e4d8573d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e4d8573d
Branch: refs/heads/master
Commit: e4d8573dcb37a069b81bade8940cdb8c9e950ecf
Parents: 7a6b7c6
Author: Daniel Pravat <dp...@outlook.com>
Authored: Wed Feb 15 16:30:42 2017 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu Feb 16 17:50:09 2017 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/abort.hpp | 16 ++++++++++++++--
3rdparty/stout/include/stout/duration.hpp | 7 +++++--
3rdparty/stout/include/stout/gzip.hpp | 4 ++--
3rdparty/stout/include/stout/os/windows/dup.hpp | 6 ++++++
3rdparty/stout/include/stout/os/windows/fd.hpp | 20 ++++++++++++++++++++
3rdparty/stout/include/stout/protobuf.hpp | 18 ++++++++++++++++--
3rdparty/stout/include/stout/windows/os.hpp | 12 ++++++++++++
7 files changed, 75 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/abort.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/abort.hpp b/3rdparty/stout/include/stout/abort.hpp
index b7dba03..d3291dc 100644
--- a/3rdparty/stout/include/stout/abort.hpp
+++ b/3rdparty/stout/include/stout/abort.hpp
@@ -42,15 +42,27 @@
inline NORETURN void _Abort(const char* prefix, const char* message)
{
+ // NOTE: On Windows, `_write` takes an `unsigned int`, not `size_t`. We
+ // preform an explicit type conversion here to silence the warning. `strlen`
+ // always returns a positive result, which means it is safe to cast it to an
+ // unsigned value.
+#ifndef __WINDOWS__
+ const size_t prefix_len = strlen(prefix);
+ const size_t message_len = strlen(message);
+#else
+ const unsigned int prefix_len = static_cast<unsigned int>(strlen(prefix));
+ const unsigned int message_len = static_cast<unsigned int>(strlen(message));
+#endif // !__WINDOWS__
+
// Write the failure message in an async-signal safe manner,
// assuming strlen is async-signal safe or optimized out.
// In fact, it is highly unlikely that strlen would be
// implemented in an unsafe manner:
// http://austingroupbugs.net/view.php?id=692
- while (::write(STDERR_FILENO, prefix, strlen(prefix)) == -1 &&
+ while (::write(STDERR_FILENO, prefix, prefix_len) == -1 &&
errno == EINTR);
while (message != nullptr &&
- ::write(STDERR_FILENO, message, strlen(message)) == -1 &&
+ ::write(STDERR_FILENO, message, message_len) == -1 &&
errno == EINTR);
// NOTE: Since `1` can be interpreted as either an `unsigned int` or a
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/duration.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/duration.hpp b/3rdparty/stout/include/stout/duration.hpp
index 0a30a09..165dd15 100644
--- a/3rdparty/stout/include/stout/duration.hpp
+++ b/3rdparty/stout/include/stout/duration.hpp
@@ -96,8 +96,11 @@ public:
struct timeval timeval() const
{
struct timeval t;
- t.tv_sec = secs();
- t.tv_usec = us() - (t.tv_sec * MILLISECONDS);
+
+ // Explicitly compute `tv_sec` and `tv_usec` instead of using `us` and
+ // `secs` to avoid converting `int64_t` -> `double` -> `long`.
+ t.tv_sec = ns() / SECONDS;
+ t.tv_usec = (ns() / MICROSECONDS) - (t.tv_sec * SECONDS / MICROSECONDS);
return t;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/gzip.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/gzip.hpp b/3rdparty/stout/include/stout/gzip.hpp
index 7040a32..1dcad5d 100644
--- a/3rdparty/stout/include/stout/gzip.hpp
+++ b/3rdparty/stout/include/stout/gzip.hpp
@@ -123,7 +123,7 @@ public:
{
stream.next_in =
const_cast<Bytef*>(reinterpret_cast<const Bytef*>(compressed.data()));
- stream.avail_in = compressed.length();
+ stream.avail_in = static_cast<uInt>(compressed.length());
// Build up the decompressed result.
Bytef buffer[GZIP_BUFFER_SIZE];
@@ -189,7 +189,7 @@ inline Try<std::string> compress(
z_stream_s stream;
stream.next_in =
const_cast<Bytef*>(reinterpret_cast<const Bytef*>(decompressed.data()));
- stream.avail_in = decompressed.length();
+ stream.avail_in = static_cast<uInt>(decompressed.length());
stream.zalloc = Z_NULL;
stream.zfree = Z_NULL;
stream.opaque = Z_NULL;
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/os/windows/dup.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/dup.hpp b/3rdparty/stout/include/stout/os/windows/dup.hpp
index 1fc4d55..1c9dda0 100644
--- a/3rdparty/stout/include/stout/os/windows/dup.hpp
+++ b/3rdparty/stout/include/stout/os/windows/dup.hpp
@@ -36,11 +36,17 @@ inline Try<WindowsFD> dup(const WindowsFD& fd)
return result;
}
case WindowsFD::FD_SOCKET: {
+#pragma warning(push)
+#pragma warning(disable : 4996)
+ // Disable compiler warning asking us to use the Unicode version of
+ // `WSASocket` and `WSADuplicateSocket`, because Mesos currently does not
+ // support Unicode. See MESOS-6817.
WSAPROTOCOL_INFO protInfo;
if (::WSADuplicateSocket(fd, GetCurrentProcessId(), &protInfo) !=
INVALID_SOCKET) {
return WSASocket(0, 0, 0, &protInfo, 0, 0);
};
+#pragma warning(pop)
return SocketError();
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/os/windows/fd.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/fd.hpp b/3rdparty/stout/include/stout/os/windows/fd.hpp
index fddaaa5..ae2db27 100644
--- a/3rdparty/stout/include/stout/os/windows/fd.hpp
+++ b/3rdparty/stout/include/stout/os/windows/fd.hpp
@@ -185,8 +185,13 @@ inline bool operator<(const WindowsFD& left, const WindowsFD& right)
if (left.crt() < 0) {
return true;
}
+#pragma warning(push)
+#pragma warning(disable : 4312)
+ // Disable `int`-to-`HANDLE` compiler warning. This is safe to do,
+ // see comment above.
return reinterpret_cast<HANDLE>(left.crt()) <
static_cast<HANDLE>(right);
+#pragma warning(pop)
}
case WindowsFD::FD_SOCKET: {
if (static_cast<SOCKET>(right) == INVALID_SOCKET) {
@@ -208,8 +213,13 @@ inline bool operator<(const WindowsFD& left, const WindowsFD& right)
if (right.crt() < 0) {
return false;
}
+#pragma warning(push)
+#pragma warning(disable : 4312)
+ // Disable `int`-to-`HANDLE` compiler warning. This is safe to do,
+ // see comment above.
return static_cast<HANDLE>(left) <
reinterpret_cast<HANDLE>(right.crt());
+#pragma warning(pop)
}
case WindowsFD::FD_HANDLE: {
return static_cast<HANDLE>(left) < static_cast<HANDLE>(right);
@@ -334,8 +344,13 @@ inline bool operator==(const WindowsFD& left, const WindowsFD& right)
if (left.crt() < 0) {
return false;
}
+#pragma warning(push)
+#pragma warning(disable : 4312)
+ // Disable `int`-to-`HANDLE` compiler warning. This is safe to do,
+ // see comment above.
return reinterpret_cast<HANDLE>(left.crt()) ==
static_cast<HANDLE>(right);
+#pragma warning(pop)
}
case WindowsFD::FD_SOCKET: {
if (static_cast<SOCKET>(right) == INVALID_SOCKET) {
@@ -357,8 +372,13 @@ inline bool operator==(const WindowsFD& left, const WindowsFD& right)
if (right.crt() < 0) {
return false;
}
+#pragma warning(push)
+#pragma warning(disable : 4312)
+ // Disable `int`-to-`HANDLE` compiler warning. This is safe to do,
+ // see comment above.
return static_cast<HANDLE>(left) ==
reinterpret_cast<HANDLE>(right.crt());
+#pragma warning(pop)
}
case WindowsFD::FD_HANDLE: {
return static_cast<HANDLE>(left) == static_cast<HANDLE>(right);
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/protobuf.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/stout/include/stout/protobuf.hpp
index c0f03bc..34290c7 100644
--- a/3rdparty/stout/include/stout/protobuf.hpp
+++ b/3rdparty/stout/include/stout/protobuf.hpp
@@ -175,7 +175,14 @@ Try<T> deserialize(const std::string& value)
T t;
(void) static_cast<google::protobuf::Message*>(&t);
- google::protobuf::io::ArrayInputStream stream(value.data(), value.size());
+ // Verify that the size of `value` fits into `ArrayInputStream`'s
+ // constructor. The maximum size of a proto2 message is 64 MB, so it is
+ // unlikely that we will hit this limit, but since an arbitrary string can be
+ // passed in, we include this check to be safe.
+ CHECK_LE(value.size(), std::numeric_limits<int>::max());
+ google::protobuf::io::ArrayInputStream stream(
+ value.data(),
+ static_cast<int>(value.size()));
if (!t.ParseFromZeroCopyStream(&stream)) {
return Error("Failed to deserialize " + t.GetDescriptor()->full_name());
}
@@ -274,8 +281,15 @@ struct Read
// must outlive the creation of ArrayInputStream.
const std::string& data = result.get();
+ // Verify that the size of `data` fits into `ArrayInputStream`'s
+ // constructor. The maximum size of a proto2 message is 64 MB, so it is
+ // unlikely that we will hit this limit, but since an arbitrary string can
+ // be passed in, we include this check to be safe.
+ CHECK_LE(data.size(), std::numeric_limits<int>::max());
T message;
- google::protobuf::io::ArrayInputStream stream(data.data(), data.size());
+ google::protobuf::io::ArrayInputStream stream(
+ data.data(),
+ static_cast<int>(data.size()));
if (!message.ParseFromZeroCopyStream(&stream)) {
if (undoFailed) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4d8573d/3rdparty/stout/include/stout/windows/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/os.hpp b/3rdparty/stout/include/stout/windows/os.hpp
index b5172fc..0bedb2d 100644
--- a/3rdparty/stout/include/stout/windows/os.hpp
+++ b/3rdparty/stout/include/stout/windows/os.hpp
@@ -51,10 +51,16 @@ inline Try<OSVERSIONINFOEX> os_version()
{
OSVERSIONINFOEX os_version;
os_version.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
+#pragma warning(push)
+#pragma warning(disable : 4996)
+ // Disable compiler warning asking us to use the Unicode version of
+ // `GetVersionEx`, because Mesos currently does not support Unicode. See
+ // MESOS-6817.
if (!::GetVersionEx(reinterpret_cast<LPOSVERSIONINFO>(&os_version))) {
return WindowsError(
"os::internal::os_version: Call to `GetVersionEx` failed");
}
+#pragma warning(pop)
return os_version;
}
@@ -459,9 +465,15 @@ inline Try<Version> release()
{
OSVERSIONINFOEX os_version;
os_version.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
+#pragma warning(push)
+#pragma warning(disable : 4996)
+ // Disable compiler warning asking us to use the Unicode version of
+ // `GetVersionEx`, because Mesos currently does not support Unicode. See
+ // MESOS-6817.
if (!::GetVersionEx(reinterpret_cast<LPOSVERSIONINFO>(&os_version))) {
return WindowsError("os::release: Call to `GetVersionEx` failed");
}
+#pragma warning(pop)
return Version(os_version.dwMajorVersion, os_version.dwMinorVersion, 0);
}