From 9b569c8ce38419fa8ad98de4db27a349ecdaf3d0 Mon Sep 17 00:00:00 2001
From: Andrey Okoshkin <okoandrey@gmail.com>
Date: Fri, 12 Jan 2018 15:59:20 +0300
Subject: [PATCH] Make Value copy constructor simplier

Helper private methods Value::dupPayload() and Value::dupMeta() are added.
Value copy constructor doesn't attempt to delete its data first.
* Value::dupPayload() duplicates a payload.
* Value::dupMeta() duplicates comments and an offset position with a limit.
---
 include/json/value.h        |  2 +
 src/lib_json/json_value.cpp | 98 ++++++++++++++++++++-----------------
 2 files changed, 54 insertions(+), 46 deletions(-)

diff --git a/include/json/value.h b/include/json/value.h
index 1643210..bcf3675 100644
--- a/include/json/value.h
+++ b/include/json/value.h
@@ -606,7 +606,9 @@ Json::Value obj_value(Json::objectValue); // {}
 
 private:
   void initBasic(ValueType type, bool allocated = false);
+  void dupPayload(const Value& other);
   void releasePayload();
+  void dupMeta(const Value& other);
 
   Value& resolveReference(const char* key);
   Value& resolveReference(const char* key, const char* end);
diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp
index eb7b6b0..30449fd 100644
--- a/src/lib_json/json_value.cpp
+++ b/src/lib_json/json_value.cpp
@@ -442,10 +442,8 @@ Value::Value(bool value) {
 }
 
 Value::Value(const Value& other) {
-  type_ = nullValue;
-  allocated_ = false;
-  comments_ = 0;
-  copy(other);
+  dupPayload(other);
+  dupMeta(other);
 }
 
 #if JSON_HAS_RVALUE_REFERENCES
@@ -481,35 +479,7 @@ void Value::swapPayload(Value& other) {
 
 void Value::copyPayload(const Value& other) {
   releasePayload();
-  type_ = other.type_;
-  allocated_ = false;
-  switch (type_) {
-  case nullValue:
-  case intValue:
-  case uintValue:
-  case realValue:
-  case booleanValue:
-    value_ = other.value_;
-    break;
-  case stringValue:
-    if (other.value_.string_ && other.allocated_) {
-      unsigned len;
-      char const* str;
-      decodePrefixedString(other.allocated_, other.value_.string_,
-          &len, &str);
-      value_.string_ = duplicateAndPrefixStringValue(str, len);
-      allocated_ = true;
-    } else {
-      value_.string_ = other.value_.string_;
-    }
-    break;
-  case arrayValue:
-  case objectValue:
-    value_.map_ = new ObjectValues(*other.value_.map_);
-    break;
-  default:
-    JSON_ASSERT_UNREACHABLE;
-  }
+  dupPayload(other);
 }
 
 void Value::swap(Value& other) {
@@ -522,19 +492,7 @@ void Value::swap(Value& other) {
 void Value::copy(const Value& other) {
   copyPayload(other);
   delete[] comments_;
-  if (other.comments_) {
-    comments_ = new CommentInfo[numberOfCommentPlacement];
-    for (int comment = 0; comment < numberOfCommentPlacement; ++comment) {
-      const CommentInfo& otherComment = other.comments_[comment];
-      if (otherComment.comment_)
-        comments_[comment].setComment(
-            otherComment.comment_, strlen(otherComment.comment_));
-    }
-  } else {
-    comments_ = 0;
-  }
-  start_ = other.start_;
-  limit_ = other.limit_;
+  dupMeta(other);
 }
 
 ValueType Value::type() const { return type_; }
@@ -1033,6 +991,38 @@ void Value::initBasic(ValueType vtype, bool allocated) {
   limit_ = 0;
 }
 
+void Value::dupPayload(const Value& other) {
+  type_ = other.type_;
+  allocated_ = false;
+  switch (type_) {
+  case nullValue:
+  case intValue:
+  case uintValue:
+  case realValue:
+  case booleanValue:
+    value_ = other.value_;
+    break;
+  case stringValue:
+    if (other.value_.string_ && other.allocated_) {
+      unsigned len;
+      char const* str;
+      decodePrefixedString(other.allocated_, other.value_.string_,
+          &len, &str);
+      value_.string_ = duplicateAndPrefixStringValue(str, len);
+      allocated_ = true;
+    } else {
+      value_.string_ = other.value_.string_;
+    }
+    break;
+  case arrayValue:
+  case objectValue:
+    value_.map_ = new ObjectValues(*other.value_.map_);
+    break;
+  default:
+    JSON_ASSERT_UNREACHABLE;
+  }
+}
+
 void Value::releasePayload() {
   switch (type_) {
   case nullValue:
@@ -1054,6 +1044,22 @@ void Value::releasePayload() {
   }
 }
 
+void Value::dupMeta(const Value& other) {
+  if (other.comments_) {
+    comments_ = new CommentInfo[numberOfCommentPlacement];
+    for (int comment = 0; comment < numberOfCommentPlacement; ++comment) {
+      const CommentInfo& otherComment = other.comments_[comment];
+      if (otherComment.comment_)
+        comments_[comment].setComment(
+            otherComment.comment_, strlen(otherComment.comment_));
+    }
+  } else {
+    comments_ = 0;
+  }
+  start_ = other.start_;
+  limit_ = other.limit_;
+}
+
 // Access an object value by name, create a null member if it does not exist.
 // @pre Type of '*this' is object or null.
 // @param key is null-terminated.