Make the Create() function comform the convention

Summary:

Moved "Return multiple values" a more conventional way.

table/block_based_table_reader.cc

@@ -148,26 +148,20 @@ class BinarySearchIndexReader : public IndexReader {
  public:
   // Read index from the file and create an intance for
   // `BinarySearchIndexReader`.
-  // The return value is a pair, where
+  // On success, index_reader will be populated; otherwise it will remain
-  //  * first element is the status indicating if the operation succeeded.
+  // unmodified.
-  //  * second element is the index reader to be created. On failure, this
+  static Status Create(RandomAccessFile* file, const BlockHandle& index_handle,
-  // element will be nullptr
+                       Env* env, const Comparator* comparator,
-  static std::pair<Status, IndexReader*> Create(RandomAccessFile* file,
+                       IndexReader** index_reader) {
-                                                const BlockHandle& index_handle,
-                                                Env* env,
-                                                const Comparator* comparator) {
     Block* index_block = nullptr;
-    auto s =
+    auto s = ReadBlockFromFile(file, ReadOptions(), index_handle,
-        ReadBlockFromFile(file, ReadOptions(), index_handle, &index_block, env);
+                               &index_block, env);
 
-    if (!s.ok()) {
+    if (s.ok()) {
-      // Logically, index_block shouldn't have been populated if any error
+      *index_reader = new BinarySearchIndexReader(comparator, index_block);
-      // occurred.
-      assert(index_block == nullptr);
-      return {s, nullptr};
     }
 
-    return {s, new BinarySearchIndexReader(comparator, index_block)};
+    return s;
   }
 
   virtual Iterator* NewIterator() override {
@@ -190,12 +184,12 @@ class BinarySearchIndexReader : public IndexReader {
 // key.
 class HashIndexReader : public IndexReader {
  public:
-  static std::pair<Status, IndexReader*> Create(
+  static Status Create(RandomAccessFile* file, const BlockHandle& index_handle,
-      RandomAccessFile* file, const BlockHandle& index_handle, Env* env,
+                       Env* env, const Comparator* comparator,
-      const Comparator* comparator, BlockBasedTable* table,
+                       BlockBasedTable* table,
-      const SliceTransform* prefix_extractor) {
+                       const SliceTransform* prefix_extractor,
-    return {Status::NotSupported("not implemented yet!"),
+                       IndexReader** index_reader) {
-            nullptr};  // not finished
+    return Status::NotSupported("not implemented yet!");
   }
 };
 
@@ -367,7 +361,7 @@ Status BlockBasedTable::Open(const Options& options, const EnvOptions& soptions,
     // and with a same life-time as this table object.
     IndexReader* index_reader = nullptr;
     // TODO: we never really verify check sum for index block
-    std::tie(s, index_reader) = new_table->CreateIndexReader();
+    s = new_table->CreateIndexReader(&index_reader);
 
     if (s.ok()) {
       rep->index_reader.reset(index_reader);
@@ -779,7 +773,7 @@ Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options)
   } else {
     // Create index reader and put it in the cache.
     Status s;
-    std::tie(s, index_reader) = CreateIndexReader();
+    s = CreateIndexReader(&index_reader);
 
     if (!s.ok()) {
       // make sure if something goes wrong, index_reader shall remain intact.
@@ -979,7 +973,7 @@ bool BlockBasedTable::TEST_KeyInCache(const ReadOptions& options,
 //  3. options
 //  4. internal_comparator
 //  5. index_type
-std::pair<Status, IndexReader*> BlockBasedTable::CreateIndexReader() const {
+Status BlockBasedTable::CreateIndexReader(IndexReader** index_reader) const {
   // Some old version of block-based tables don't have index type present in
   // table properties. If that's the case we can safely use the kBinarySearch.
   auto index_type = BlockBasedTableOptions::kBinarySearch;
@@ -994,15 +988,14 @@ std::pair<Status, IndexReader*> BlockBasedTable::CreateIndexReader() const {
     case BlockBasedTableOptions::kBinarySearch: {
       return BinarySearchIndexReader::Create(
           rep_->file.get(), rep_->index_handle, rep_->options.env,
-          &rep_->internal_comparator);
+          &rep_->internal_comparator, index_reader);
     }
     default: {
       std::string error_message =
           "Unrecognized index type: " + std::to_string(rep_->index_type);
       // equivalent to assert(false), but more informative.
       assert(!error_message.c_str());
-      return {Status::InvalidArgument(error_message.c_str()),
+      return Status::InvalidArgument(error_message.c_str());
-              nullptr};  // cannot reach here
     }
   }
 }

table/block_based_table_reader.h

@@ -164,7 +164,7 @@ class BlockBasedTable : public TableReader {
 
   void ReadMeta(const Footer& footer);
   void ReadFilter(const Slice& filter_handle_value);
-  std::pair<Status, IndexReader*> CreateIndexReader() const;
+  Status CreateIndexReader(IndexReader** index_reader) const;
 
   // Read the meta block from sst.
   static Status ReadMetaBlock(