kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wdberke...@apache.org
Subject kudu git commit: Fix jsonreader signed int extraction and add unsigned extraction
Date Tue, 16 Oct 2018 17:40:13 GMT
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 <awong@cloudera.com>
Tested-by: Will Berkeley <wdberkeley@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 <wdberkeley@gmail.org>
Authored: Mon Oct 15 00:27:51 2018 -0700
Committer: Will Berkeley <wdberkeley@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;


Mime
View raw message