From 234f33a3f9154f2d81ef72bd0165e1bf064c2c5a Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <ajkr@users.noreply.github.com>
Date: Wed, 23 Aug 2017 10:45:17 -0700
Subject: [PATCH] allow nullptr Slice only as sentinel

Summary:
Allow `Slice` holding nullptr as a sentinel value but not in comparisons. This new restriction eliminates the need for the manual checks in 39ef900551a4d88c8546ca086baaba76730e6162, while still conforming to glibc's `memcmp` API. Thanks siying for the idea. Users may need to migrate, so mentioned it in HISTORY.md.
Closes https://github.com/facebook/rocksdb/pull/2777

Differential Revision: D5686016

Pulled By: ajkr

fbshipit-source-id: 03a2ca3fd9a0ebade9d0d5686c81d59a9534f563
---
 HISTORY.md                         |  1 +
 include/rocksdb/slice.h            | 10 +---------
 table/cuckoo_table_builder_test.cc |  6 +++++-
 utilities/merge_operators/max.cc   |  2 ++
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index a40a3b8926..a63d9d6286 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -2,6 +2,7 @@
 ## Unreleased
 ### Public API Change
 * Users of `Statistics::getHistogramString()` will see fewer histogram buckets and different bucket endpoints.
+* `Slice::compare` and BytewiseComparator `Compare` no longer accept `Slice`s containing nullptr.
 
 ### New Features
 * Add Iterator::Refresh(), which allows users to update the iterator state so that they can avoid some initialization costs of recreating iterators.
diff --git a/include/rocksdb/slice.h b/include/rocksdb/slice.h
index ec33c97e63..1630803b9f 100644
--- a/include/rocksdb/slice.h
+++ b/include/rocksdb/slice.h
@@ -214,16 +214,8 @@ inline bool operator!=(const Slice& x, const Slice& y) {
 }
 
 inline int Slice::compare(const Slice& b) const {
-  const size_t min_len = (size_ < b.size_) ? size_ : b.size_;
-  if (min_len == 0) {
-    if (size_ > 0) {
-      return 1;
-    } else if (b.size_ > 0) {
-      return -1;
-    }
-    return 0;
-  }
   assert(data_ != nullptr && b.data_ != nullptr);
+  const size_t min_len = (size_ < b.size_) ? size_ : b.size_;
   int r = memcmp(data_, b.data_, min_len);
   if (r == 0) {
     if (size_ < b.size_) r = -1;
diff --git a/table/cuckoo_table_builder_test.cc b/table/cuckoo_table_builder_test.cc
index ec282b4b54..93daaca472 100644
--- a/table/cuckoo_table_builder_test.cc
+++ b/table/cuckoo_table_builder_test.cc
@@ -109,7 +109,11 @@ class CuckooBuilderTest : public testing::Test {
           expected_locations.begin();
       if (key_idx == keys.size()) {
         // i is not one of the expected locations. Empty bucket.
-        ASSERT_EQ(read_slice.compare(expected_unused_bucket), 0);
+        if (read_slice.data() == nullptr) {
+          ASSERT_EQ(0, expected_unused_bucket.size());
+        } else {
+          ASSERT_EQ(read_slice.compare(expected_unused_bucket), 0);
+        }
       } else {
         keys_found[key_idx] = true;
         ASSERT_EQ(read_slice.compare(keys[key_idx] + values[key_idx]), 0);
diff --git a/utilities/merge_operators/max.cc b/utilities/merge_operators/max.cc
index 06e233fe89..5f42e816ef 100644
--- a/utilities/merge_operators/max.cc
+++ b/utilities/merge_operators/max.cc
@@ -25,6 +25,8 @@ class MaxOperator : public MergeOperator {
     if (merge_in.existing_value) {
       max = Slice(merge_in.existing_value->data(),
                   merge_in.existing_value->size());
+    } else if (max.data() == nullptr) {
+      max = Slice();
     }
 
     for (const auto& op : merge_in.operand_list) {