Fix rocksdb::Status::getState

Summary:
This fixes the Java API for Status#getState use in Native code and also simplifies the implementation of rocksdb::Status::getState.
Closes https://github.com/facebook/rocksdb/issues/1688
Closes https://github.com/facebook/rocksdb/pull/1714

Differential Revision: D4364181

Pulled By: yiwu-arbug

fbshipit-source-id: 8e073b4

db/db_test.cc

@@ -197,6 +197,19 @@ TEST_F(DBTest, MemEnvTest) {
 }
 #endif  // ROCKSDB_LITE
 
+TEST_F(DBTest, OpenWhenOpen) {
+  Options options = CurrentOptions();
+  options.env = env_;
+  rocksdb::DB* db2 = nullptr;
+  rocksdb::Status s = DB::Open(options, dbname_, &db2);
+
+  ASSERT_EQ(Status::Code::kIOError, s.code());
+  ASSERT_EQ(Status::SubCode::kNone, s.subcode());
+  ASSERT_TRUE(strncmp("lock ", s.getState(), 5) == 0);
+
+  delete db2;
+}
+
 TEST_F(DBTest, WriteEmptyBatch) {
   Options options = CurrentOptions();
   options.env = env_;

include/rocksdb/status.h

@@ -75,6 +75,7 @@ class Status {
 
   SubCode subcode() const { return subcode_; }
 
+  // Returns a C style string indicating the message of the Status
   const char* getState() const { return state_; }
 
   // Return a success status.

java/rocksjni/portal.h

@@ -120,8 +120,8 @@ class StatusJni : public RocksDBNativeClass<rocksdb::Status*, StatusJni> {
 
     jstring jstate = nullptr;
     if (status.getState() != nullptr) {
-      std::string s(status.getState());
-      jstate = env->NewStringUTF(s.c_str());
+      const char* const state = status.getState();
+      jstate = env->NewStringUTF(state);
     }
     return env->NewObject(getJClass(env), mid, toJavaStatusCode(status.code()),
                           toJavaStatusSubCode(status.subcode()), jstate);

java/src/test/java/org/rocksdb/RocksDBTest.java

@@ -12,6 +12,7 @@ import org.junit.rules.TemporaryFolder;
 import java.util.*;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
 
 public class RocksDBTest {
 
@@ -42,6 +43,21 @@ public class RocksDBTest {
     }
   }
 
+  @Test
+  public void openWhenOpen() throws RocksDBException {
+    final String dbPath = dbFolder.getRoot().getAbsolutePath();
+
+    try (final RocksDB db1 = RocksDB.open(dbPath)) {
+      try (final RocksDB db2 = RocksDB.open(dbPath)) {
+        fail("Should have thrown an exception when opening the same db twice");
+      } catch (final RocksDBException e) {
+        assertThat(e.getStatus().getCode()).isEqualTo(Status.Code.IOError);
+        assertThat(e.getStatus().getSubCode()).isEqualTo(Status.SubCode.None);
+        assertThat(e.getStatus().getState()).startsWith("lock ");
+      }
+    }
+  }
+
   @Test
   public void put() throws RocksDBException {
     try (final RocksDB db = RocksDB.open(dbFolder.getRoot().getAbsolutePath());

util/status.cc

@@ -7,17 +7,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
+#include "rocksdb/status.h"
 #include <stdio.h>
+#include <cstring>
 #include "port/port.h"
-#include "rocksdb/status.h"
 
 namespace rocksdb {
 
 const char* Status::CopyState(const char* state) {
-  uint32_t size;
-  memcpy(&size, state, sizeof(size));
-  char* result = new char[size + 4];
-  memcpy(result, state, size + 4);
+  char* const result =
+      new char[std::strlen(state) + 1];  // +1 for the null terminator
+  std::strcpy(result, state);
   return result;
 }
 
@@ -25,17 +25,17 @@ Status::Status(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2
     : code_(_code), subcode_(_subcode) {
   assert(code_ != kOk);
   assert(subcode_ != kMaxSubCode);
-  const uint32_t len1 = static_cast<uint32_t>(msg.size());
-  const uint32_t len2 = static_cast<uint32_t>(msg2.size());
-  const uint32_t size = len1 + (len2 ? (2 + len2) : 0);
-  char* result = new char[size + 4];
-  memcpy(result, &size, sizeof(size));
-  memcpy(result + 4, msg.data(), len1);
+  const size_t len1 = msg.size();
+  const size_t len2 = msg2.size();
+  const size_t size = len1 + (len2 ? (2 + len2) : 0);
+  char* const result = new char[size + 1];  // +1 for null terminator
+  memcpy(result, msg.data(), len1);
   if (len2) {
-    result[4 + len1] = ':';
-    result[5 + len1] = ' ';
-    memcpy(result + 6 + len1, msg2.data(), len2);
+    result[len1] = ':';
+    result[len1 + 1] = ' ';
+    memcpy(result + len1 + 2, msg2.data(), len2);
   }
+  result[size] = '\0';  // null terminator for C style string
   state_ = result;
 }
 
@@ -98,9 +98,7 @@ std::string Status::ToString() const {
   }
 
   if (state_ != nullptr) {
-    uint32_t length;
-    memcpy(&length, state_, sizeof(length));
-    result.append(state_ + 4, length);
+    result.append(state_);
   }
   return result;
 }