You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by wd...@apache.org on 2018/10/16 17:40:13 UTC
kudu git commit: Fix jsonreader signed int extraction and add
unsigned extraction
Repository: kudu
Updated Branches:
refs/heads/master 614b446e1 -> 02e82ca14
Fix jsonreader signed int extraction and add unsigned extraction
JsonReader::GetInt64's implementation was actually of
JsonReader::GetUint64, and similarly for JsonReader::GetInt. This patch
corrects the error and adds JsonReader::{GetUint32, GetUint64} too.
A new test ensures that the 4 JsonReader::Get[Ui|I]nt[32|64] methods
work as expected on signed and unsigned values.
Change-Id: I0b07757bacc756643aea5613b3491a34cb051a43
Reviewed-on: http://gerrit.cloudera.org:8080/11683
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/02e82ca1
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/02e82ca1
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/02e82ca1
Branch: refs/heads/master
Commit: 02e82ca14f5c3c89f201224ea5b18dc0052ab835
Parents: 614b446
Author: Will Berkeley <wd...@gmail.org>
Authored: Mon Oct 15 00:27:51 2018 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Tue Oct 16 17:39:10 2018 +0000
----------------------------------------------------------------------
src/kudu/util/jsonreader-test.cc | 104 ++++++++++++++++++++++++++++++++++
src/kudu/util/jsonreader.cc | 27 +++++++++
src/kudu/util/jsonreader.h | 8 +++
3 files changed, 139 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/02e82ca1/src/kudu/util/jsonreader-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader-test.cc b/src/kudu/util/jsonreader-test.cc
index 9f62c31..50e1fc6 100644
--- a/src/kudu/util/jsonreader-test.cc
+++ b/src/kudu/util/jsonreader-test.cc
@@ -18,9 +18,11 @@
#include "kudu/util/jsonreader.h"
#include <cstdint>
+#include <limits>
#include <string>
#include <vector>
+#include <glog/logging.h> // IWYU pragma: keep
#include <gtest/gtest.h>
#include <rapidjson/document.h>
@@ -126,6 +128,108 @@ TEST(JsonReaderTest, LessBasic) {
ASSERT_TRUE(r.ExtractObjectArray(r.root(), "bool", nullptr).IsInvalidArgument());
}
+TEST(JsonReaderTest, SignedAndUnsignedInts) {
+ // The rapidjson code has some improper handling of the min int32 and min
+ // int64 that exposes UB.
+ #if defined(ADDRESS_SANITIZER)
+ LOG(WARNING) << "this test is skipped in ASAN builds";
+ return;
+ #endif
+
+ constexpr auto kMaxInt32 = std::numeric_limits<int32_t>::max();
+ constexpr auto kMaxInt64 = std::numeric_limits<int64_t>::max();
+ constexpr auto kMaxUint32 = std::numeric_limits<uint32_t>::max();
+ constexpr auto kMaxUint64 = std::numeric_limits<uint64_t>::max();
+ constexpr auto kMinInt32 = std::numeric_limits<int32_t>::min();
+ constexpr auto kMinInt64 = std::numeric_limits<int64_t>::min();
+ const string doc = Substitute(
+ "{ \"negative\" : -1, \"signed_big32\" : $0, \"signed_big64\" : $1, "
+ "\"unsigned_big32\" : $2, \"unsigned_big64\" : $3, "
+ "\"signed_small32\" : $4, \"signed_small64\" : $5 }",
+ kMaxInt32, kMaxInt64, kMaxUint32, kMaxUint64, kMinInt32, kMinInt64);
+ JsonReader r(doc);
+ ASSERT_OK(r.Init());
+
+ // -1.
+ const char* const negative = "negative";
+ int32_t negative32;
+ ASSERT_OK(r.ExtractInt32(r.root(), negative, &negative32));
+ ASSERT_EQ(-1, negative32);
+ int64_t negative64;
+ ASSERT_OK(r.ExtractInt64(r.root(), negative, &negative64));
+ ASSERT_EQ(-1, negative64);
+ ASSERT_TRUE(r.ExtractUint32(r.root(), negative, nullptr).IsInvalidArgument());
+ ASSERT_TRUE(r.ExtractUint64(r.root(), negative, nullptr).IsInvalidArgument());
+
+ // Max signed 32-bit integer.
+ const char* const signed_big32 = "signed_big32";
+ int32_t signed_big32_int32;
+ ASSERT_OK(r.ExtractInt32(r.root(), signed_big32, &signed_big32_int32));
+ ASSERT_EQ(kMaxInt32, signed_big32_int32);
+ int64_t signed_big32_int64;
+ ASSERT_OK(r.ExtractInt64(r.root(), signed_big32, &signed_big32_int64));
+ ASSERT_EQ(kMaxInt32, signed_big32_int64);
+ uint32_t signed_big32_uint32;
+ ASSERT_OK(r.ExtractUint32(r.root(), signed_big32, &signed_big32_uint32));
+ ASSERT_EQ(kMaxInt32, signed_big32_uint32);
+ uint64_t signed_big32_uint64;
+ ASSERT_OK(r.ExtractUint64(r.root(), signed_big32, &signed_big32_uint64));
+ ASSERT_EQ(kMaxInt32, signed_big32_uint64);
+
+ // Max signed 64-bit integer.
+ const char* const signed_big64 = "signed_big64";
+ ASSERT_TRUE(r.ExtractInt32(r.root(), signed_big64, nullptr).IsInvalidArgument());
+ int64_t signed_big64_int64;
+ ASSERT_OK(r.ExtractInt64(r.root(), signed_big64, &signed_big64_int64));
+ ASSERT_EQ(kMaxInt64, signed_big64_int64);
+ ASSERT_TRUE(r.ExtractUint32(r.root(), signed_big64, nullptr).IsInvalidArgument());
+ uint64_t signed_big64_uint64;
+ ASSERT_OK(r.ExtractUint64(r.root(), signed_big64, &signed_big64_uint64));
+ ASSERT_EQ(kMaxInt64, signed_big64_uint64);
+
+ // Max unsigned 32-bit integer.
+ const char* const unsigned_big32 = "unsigned_big32";
+ ASSERT_TRUE(r.ExtractInt32(r.root(), unsigned_big32, nullptr).IsInvalidArgument());
+ int64_t unsigned_big32_int64;
+ ASSERT_OK(r.ExtractInt64(r.root(), unsigned_big32, &unsigned_big32_int64));
+ ASSERT_EQ(kMaxUint32, unsigned_big32_int64);
+ uint32_t unsigned_big32_uint32;
+ ASSERT_OK(r.ExtractUint32(r.root(), unsigned_big32, &unsigned_big32_uint32));
+ ASSERT_EQ(kMaxUint32, unsigned_big32_uint32);
+ uint64_t unsigned_big32_uint64;
+ ASSERT_OK(r.ExtractUint64(r.root(), unsigned_big32, &unsigned_big32_uint64));
+ ASSERT_EQ(kMaxUint32, unsigned_big32_uint64);
+
+ // Max unsigned 64-bit integer.
+ const char* const unsigned_big64 = "unsigned_big64";
+ ASSERT_TRUE(r.ExtractInt32(r.root(), unsigned_big64, nullptr).IsInvalidArgument());
+ ASSERT_TRUE(r.ExtractInt64(r.root(), unsigned_big64, nullptr).IsInvalidArgument());
+ ASSERT_TRUE(r.ExtractUint32(r.root(), unsigned_big64, nullptr).IsInvalidArgument());
+ uint64_t unsigned_big64_uint64;
+ ASSERT_OK(r.ExtractUint64(r.root(), unsigned_big64, &unsigned_big64_uint64));
+ ASSERT_EQ(kMaxUint64, unsigned_big64_uint64);
+
+ // Min signed 32-bit integer.
+ const char* const signed_small32 = "signed_small32";
+ int32_t small32_int32;
+ ASSERT_OK(r.ExtractInt32(r.root(), signed_small32, &small32_int32));
+ ASSERT_EQ(kMinInt32, small32_int32);
+ int64_t small32_int64;
+ ASSERT_OK(r.ExtractInt64(r.root(), signed_small32, &small32_int64));
+ ASSERT_EQ(kMinInt32, small32_int64);
+ ASSERT_TRUE(r.ExtractUint32(r.root(), signed_small32, nullptr).IsInvalidArgument());
+ ASSERT_TRUE(r.ExtractUint64(r.root(), signed_small32, nullptr).IsInvalidArgument());
+
+ // Min signed 32-bit integer.
+ const char* const signed_small64 = "signed_small64";
+ ASSERT_TRUE(r.ExtractInt32(r.root(), signed_small64, nullptr).IsInvalidArgument());
+ int64_t small64_int64;
+ ASSERT_OK(r.ExtractInt64(r.root(), signed_small64, &small64_int64));
+ ASSERT_EQ(kMinInt64, small64_int64);
+ ASSERT_TRUE(r.ExtractUint32(r.root(), signed_small64, nullptr).IsInvalidArgument());
+ ASSERT_TRUE(r.ExtractUint64(r.root(), signed_small64, nullptr).IsInvalidArgument());
+}
+
TEST(JsonReaderTest, Objects) {
JsonReader r("{ \"foo\" : { \"1\" : 1 } }");
ASSERT_OK(r.Init());
http://git-wip-us.apache.org/repos/asf/kudu/blob/02e82ca1/src/kudu/util/jsonreader.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader.cc b/src/kudu/util/jsonreader.cc
index acbc869..969d123 100644
--- a/src/kudu/util/jsonreader.cc
+++ b/src/kudu/util/jsonreader.cc
@@ -66,6 +66,20 @@ Status JsonReader::ExtractInt32(const Value* object,
"Wrong type during field extraction: expected int32 but got $0",
val->GetType()));
}
+ *result = val->GetInt();
+ return Status::OK();
+}
+
+Status JsonReader::ExtractUint32(const Value* object,
+ const char* field,
+ uint32_t* result) const {
+ const Value* val;
+ RETURN_NOT_OK(ExtractField(object, field, &val));
+ if (PREDICT_FALSE(!val->IsUint())) {
+ return Status::InvalidArgument(Substitute(
+ "Wrong type during field extraction: expected uint32 but got $0",
+ val->GetType()));
+ }
*result = val->GetUint();
return Status::OK();
}
@@ -79,6 +93,19 @@ Status JsonReader::ExtractInt64(const Value* object,
return Status::InvalidArgument(Substitute(
"Wrong type during field extraction: expected int64 but got $0",
val->GetType())); }
+ *result = val->GetInt64();
+ return Status::OK();
+}
+
+Status JsonReader::ExtractUint64(const Value* object,
+ const char* field,
+ uint64_t* result) const {
+ const Value* val;
+ RETURN_NOT_OK(ExtractField(object, field, &val));
+ if (PREDICT_FALSE(!val->IsUint64())) {
+ return Status::InvalidArgument(Substitute(
+ "Wrong type during field extraction: expected uint64 but got $0",
+ val->GetType())); }
*result = val->GetUint64();
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/02e82ca1/src/kudu/util/jsonreader.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/jsonreader.h b/src/kudu/util/jsonreader.h
index e389b57..2dd3651 100644
--- a/src/kudu/util/jsonreader.h
+++ b/src/kudu/util/jsonreader.h
@@ -56,10 +56,18 @@ class JsonReader {
const char* field,
int32_t* result) const;
+ Status ExtractUint32(const rapidjson::Value* object,
+ const char* field,
+ uint32_t* result) const;
+
Status ExtractInt64(const rapidjson::Value* object,
const char* field,
int64_t* result) const;
+ Status ExtractUint64(const rapidjson::Value* object,
+ const char* field,
+ uint64_t* result) const;
+
Status ExtractString(const rapidjson::Value* object,
const char* field,
std::string* result) const;