You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2018/06/14 14:13:24 UTC
mesos git commit: Removed static variable with non-constant
initialization.
Repository: mesos
Updated Branches:
refs/heads/master e25385a2f -> 4189657d7
Removed static variable with non-constant initialization.
The static variable was initialized using a dynamic memory
allocation. This is against our coding style, and makes it
as easy as writing
static process::http::NotFound DEFAULT_RESPONSE;
for a user to introduce a potential segfault into his code.
Review: https://reviews.apache.org/r/67525/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4189657d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4189657d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4189657d
Branch: refs/heads/master
Commit: 4189657d7e63ccc74f814ab82440ad47eba6cf11
Parents: e25385a
Author: Benno Evers <be...@mesosphere.com>
Authored: Thu Jun 14 16:13:01 2018 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu Jun 14 16:13:01 2018 +0200
----------------------------------------------------------------------
3rdparty/libprocess/include/process/http.hpp | 5 ++-
3rdparty/libprocess/src/decoder.hpp | 4 +--
3rdparty/libprocess/src/http.cpp | 39 +++++++++++++++++++++--
3rdparty/libprocess/src/tests/http_tests.cpp | 6 ++++
4 files changed, 46 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4189657d/3rdparty/libprocess/include/process/http.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp
index 055447e..fa66b0f 100644
--- a/3rdparty/libprocess/include/process/http.hpp
+++ b/3rdparty/libprocess/include/process/http.hpp
@@ -116,9 +116,8 @@ void unsetCallbacks();
} // namespace authorization {
-// Status code reason strings, from the HTTP1.1 RFC:
-// http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html
-extern hashmap<uint16_t, std::string>* statuses;
+// Checks if the given status code is defined by RFC 2616.
+bool isValidStatus(uint16_t code);
// Represents a Uniform Resource Locator:
// scheme://domain|ip:port/path?query#fragment
http://git-wip-us.apache.org/repos/asf/mesos/blob/4189657d/3rdparty/libprocess/src/decoder.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/decoder.hpp b/3rdparty/libprocess/src/decoder.hpp
index a078435..55cd695 100644
--- a/3rdparty/libprocess/src/decoder.hpp
+++ b/3rdparty/libprocess/src/decoder.hpp
@@ -445,7 +445,7 @@ private:
CHECK_NOTNULL(decoder->response);
- if (http::statuses->contains(decoder->parser.status_code)) {
+ if (http::isValidStatus(decoder->parser.status_code)) {
decoder->response->code = decoder->parser.status_code;
decoder->response->status =
@@ -685,7 +685,7 @@ private:
decoder->field.clear();
decoder->value.clear();
- if (http::statuses->contains(decoder->parser.status_code)) {
+ if (http::isValidStatus(decoder->parser.status_code)) {
decoder->response->code = decoder->parser.status_code;
decoder->response->status =
http://git-wip-us.apache.org/repos/asf/mesos/blob/4189657d/3rdparty/libprocess/src/http.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp
index 9fd5ea0..dc38716 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -86,8 +86,15 @@ using process::network::internal::SocketImpl;
namespace process {
namespace http {
+struct StatusDescription {
+ uint16_t code;
+ const char* description;
+};
+
-hashmap<uint16_t, string>* statuses = new hashmap<uint16_t, string> {
+// Status code reason strings, from the HTTP1.1 RFC:
+// http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html
+StatusDescription statuses[] = {
{100, "100 Continue"},
{101, "101 Switching Protocols"},
{200, "200 OK"},
@@ -173,10 +180,36 @@ const uint16_t Status::GATEWAY_TIMEOUT = 504;
const uint16_t Status::HTTP_VERSION_NOT_SUPPORTED = 505;
+// Since the status codes are stored in increasing order, we could also
+// use std::lower_bound to do the lookup with logarithmic complexity.
+// However, according to some cursory research, on most CPUs this will
+// be slower until the array size is around 100 elemnts.
+//
+// [1]: https://schani.wordpress.com/2010/04/30/linear-vs-binary-search/
+// [2]: https://stackoverflow.com/questions/1275665/at-which-n-does-binary-search-become-faster-than-linear-search-on-a-modern-cpu
+
string Status::string(uint16_t code)
{
- return http::statuses->get(code)
- .getOrElse(stringify(code));
+ auto value = std::find_if(
+ std::begin(statuses),
+ std::end(statuses),
+ [code](const StatusDescription& sd) { return sd.code == code; });
+
+ if (value != std::end(statuses)) {
+ return value->description;
+ }
+
+ // Fallback for unknown status codes.
+ return stringify(code);
+}
+
+
+bool isValidStatus(uint16_t code)
+{
+ return std::end(statuses) != std::find_if(
+ std::begin(statuses),
+ std::end(statuses),
+ [code](const StatusDescription& sd) { return sd.code == code; });
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4189657d/3rdparty/libprocess/src/tests/http_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp
index ca1ea11..5429034 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -231,6 +231,12 @@ INSTANTIATE_TEST_CASE_P(
// TODO(vinod): Use AWAIT_EXPECT_RESPONSE_STATUS_EQ in the tests.
+TEST_P(HTTPTest, Statuses)
+{
+ EXPECT_TRUE(process::http::isValidStatus(200));
+ EXPECT_TRUE(process::http::isValidStatus(404));
+ EXPECT_FALSE(process::http::isValidStatus(1337));
+}
TEST_P(HTTPTest, Endpoints)
{