Remove ldb HexToString method's usage of sscanf

Summary:
Fix hex2String performance issues by removing sscanf dependency.
Also fixed some edge case handling (odd length, bad input).

Test Plan: Created a test file which called old and new implementation, and validated results are the same. I'll paste results in the phabricator diff.

Reviewers: igor, rven, anthony, IslamAbdelRahman, kradhakrishnan, yhchiang, sdong

Reviewed By: sdong

Subscribers: thatsafunnyname, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D46785

CMakeLists.txt

@@ -322,6 +322,7 @@ set(TESTS
         util/file_reader_writer_test.cc
         util/heap_test.cc
         util/histogram_test.cc
+        util/ldb_cmd_test.cc
         util/manual_compaction_test.cc
         util/memenv_test.cc
         util/mock_env_test.cc

Makefile

@@ -306,7 +306,8 @@ TESTS = \
 	heap_test \
 	compact_on_deletion_collector_test \
 	compaction_job_stats_test \
-	transaction_test
+	transaction_test \
+	ldb_cmd_test
 
 SUBSET :=  $(shell echo $(TESTS) |sed s/^.*$(ROCKSDBTESTS_START)/$(ROCKSDBTESTS_START)/)
 
@@ -933,6 +934,9 @@ transaction_test: utilities/transactions/transaction_test.o $(LIBOBJECTS) $(TEST
 sst_dump: tools/sst_dump.o $(LIBOBJECTS)
 	$(AM_LINK)
 
+ldb_cmd_test: util/ldb_cmd_test.o $(LIBOBJECTS) $(TESTHARNESS)
+	$(AM_LINK)
+
 ldb: tools/ldb.o $(LIBOBJECTS)
 	$(AM_LINK)
 

src.mk

@@ -259,7 +259,8 @@ TEST_BENCH_SOURCES =                                                    \
   util/testharness.cc                                                   \
   util/testutil.cc                                                      \
   util/thread_list_test.cc                                              \
-  util/thread_local_test.cc
+  util/thread_local_test.cc                                             \
+  util/ldb_cmd_test.cc
 
 JNI_NATIVE_SOURCES =                                          \
   java/rocksjni/backupenginejni.cc                            \

util/ldb_cmd.h

@@ -130,18 +130,38 @@ public:
   }
 
   static string HexToString(const string& str) {
+    std::string::size_type len = str.length();
     string parsed;
-    if (str[0] != '0' || str[1] != 'x') {
+    static const char* const hexas = "0123456789ABCDEF";
+    parsed.reserve(len / 2);
+
+    if (len < 2 || str[0] != '0' || str[1] != 'x') {
       fprintf(stderr, "Invalid hex input %s.  Must start with 0x\n",
               str.c_str());
       throw "Invalid hex input";
     }
 
-    for (unsigned int i = 2; i < str.length();) {
+    for (unsigned int i = 2; i < len; i += 2) {
-      int c;
+      char a = static_cast<char>(toupper(str[i]));
-      sscanf(str.c_str() + i, "%2X", &c);
+      const char* p = std::lower_bound(hexas, hexas + 16, a);
-      parsed.push_back(c);
+      if (*p != a) {
-      i += 2;
+        throw "Invalid hex value";
+      }
+
+      if (i + 1 >= len) {
+        // if odd number of chars than we just hit end of string
+        parsed.push_back(p - hexas);
+        break;
+      }
+
+      char b = static_cast<char>(toupper(str[i + 1]));
+      const char* q = std::lower_bound(hexas, hexas + 16, b);
+      if (*q == b) {
+        // pairwise compute decimal value from hex
+        parsed.push_back(((p - hexas) << 4) | (q - hexas));
+      } else {
+        throw "Invalid hex value";
+      }
     }
     return parsed;
   }

util/ldb_cmd_test.cc

@@ -0,0 +1,44 @@
+//  Copyright (c) 2013, Facebook, Inc.  All rights reserved.
+//  This source code is licensed under the BSD-style license found in the
+//  LICENSE file in the root directory of this source tree. An additional grant
+//  of patent rights can be found in the PATENTS file in the same directory.
+//
+#include "util/ldb_cmd.h"
+#include "util/testharness.h"
+
+class LdbCmdTest : public testing::Test {};
+
+TEST_F(LdbCmdTest, HexToString) {
+  // map input to expected outputs.
+  map<string, vector<int>> inputMap = {
+      {"0x7", {7}},         {"0x5050", {80, 80}}, {"0xFF", {-1}},
+      {"0x1234", {18, 52}}, {"0xaa", {-86}}, {"0x123", {18, 3}},
+  };
+
+  for (const auto& inPair : inputMap) {
+    auto actual = rocksdb::LDBCommand::HexToString(inPair.first);
+    auto expected = inPair.second;
+    for (unsigned int i = 0; i < actual.length(); i++) {
+      ASSERT_EQ(expected[i], static_cast<int>(actual[i]));
+    }
+  }
+}
+
+TEST_F(LdbCmdTest, HexToStringBadInputs) {
+  const vector<string> badInputs = {
+      "0xZZ", "123", "0xx5", "0x11G", "Ox12", "0xT", "0x1Q1",
+  };
+  for (const auto badInput : badInputs) {
+    try {
+      rocksdb::LDBCommand::HexToString(badInput);
+      std::cerr << "Should fail on bad hex value: " << badInput << "\n";
+      FAIL();
+    } catch (...) {
+    }
+  }
+}
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

util/sst_dump_tool.cc

@@ -329,23 +329,6 @@ void print_help() {
           " [--show_compression_sizes [--set_block_size=<block_size>]]\n");
 }
 
-string HexToString(const string& str) {
-  string parsed;
-  if (str[0] != '0' || str[1] != 'x') {
-    fprintf(stderr, "Invalid hex input %s.  Must start with 0x\n",
-            str.c_str());
-    throw "Invalid hex input";
-  }
-
-  for (unsigned int i = 2; i < str.length();) {
-    int c;
-    sscanf(str.c_str() + i, "%2X", &c);
-    parsed.push_back(c);
-    i += 2;
-  }
-  return parsed;
-}
-
 }  // namespace
 
 int SSTDumpTool::Run(int argc, char** argv) {
@@ -409,10 +392,10 @@ int SSTDumpTool::Run(int argc, char** argv) {
 
   if (input_key_hex) {
     if (has_from) {
-      from_key = HexToString(from_key);
+      from_key = rocksdb::LDBCommand::HexToString(from_key);
     }
     if (has_to) {
-      to_key = HexToString(to_key);
+      to_key = rocksdb::LDBCommand::HexToString(to_key);
     }
   }