Skip to content

Commit 9b569c8

Browse files
committed
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.
1 parent 392e3a5 commit 9b569c8

File tree

2 files changed

+54
-46
lines changed

2 files changed

+54
-46
lines changed

include/json/value.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,9 @@ Json::Value obj_value(Json::objectValue); // {}
606606

607607
private:
608608
void initBasic(ValueType type, bool allocated = false);
609+
void dupPayload(const Value& other);
609610
void releasePayload();
611+
void dupMeta(const Value& other);
610612

611613
Value& resolveReference(const char* key);
612614
Value& resolveReference(const char* key, const char* end);

src/lib_json/json_value.cpp

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,8 @@ Value::Value(bool value) {
442442
}
443443

444444
Value::Value(const Value& other) {
445-
type_ = nullValue;
446-
allocated_ = false;
447-
comments_ = 0;
448-
copy(other);
445+
dupPayload(other);
446+
dupMeta(other);
449447
}
450448

451449
#if JSON_HAS_RVALUE_REFERENCES
@@ -481,35 +479,7 @@ void Value::swapPayload(Value& other) {
481479

482480
void Value::copyPayload(const Value& other) {
483481
releasePayload();
484-
type_ = other.type_;
485-
allocated_ = false;
486-
switch (type_) {
487-
case nullValue:
488-
case intValue:
489-
case uintValue:
490-
case realValue:
491-
case booleanValue:
492-
value_ = other.value_;
493-
break;
494-
case stringValue:
495-
if (other.value_.string_ && other.allocated_) {
496-
unsigned len;
497-
char const* str;
498-
decodePrefixedString(other.allocated_, other.value_.string_,
499-
&len, &str);
500-
value_.string_ = duplicateAndPrefixStringValue(str, len);
501-
allocated_ = true;
502-
} else {
503-
value_.string_ = other.value_.string_;
504-
}
505-
break;
506-
case arrayValue:
507-
case objectValue:
508-
value_.map_ = new ObjectValues(*other.value_.map_);
509-
break;
510-
default:
511-
JSON_ASSERT_UNREACHABLE;
512-
}
482+
dupPayload(other);
513483
}
514484

515485
void Value::swap(Value& other) {
@@ -522,19 +492,7 @@ void Value::swap(Value& other) {
522492
void Value::copy(const Value& other) {
523493
copyPayload(other);
524494
delete[] comments_;
525-
if (other.comments_) {
526-
comments_ = new CommentInfo[numberOfCommentPlacement];
527-
for (int comment = 0; comment < numberOfCommentPlacement; ++comment) {
528-
const CommentInfo& otherComment = other.comments_[comment];
529-
if (otherComment.comment_)
530-
comments_[comment].setComment(
531-
otherComment.comment_, strlen(otherComment.comment_));
532-
}
533-
} else {
534-
comments_ = 0;
535-
}
536-
start_ = other.start_;
537-
limit_ = other.limit_;
495+
dupMeta(other);
538496
}
539497

540498
ValueType Value::type() const { return type_; }
@@ -1033,6 +991,38 @@ void Value::initBasic(ValueType vtype, bool allocated) {
1033991
limit_ = 0;
1034992
}
1035993

994+
void Value::dupPayload(const Value& other) {
995+
type_ = other.type_;
996+
allocated_ = false;
997+
switch (type_) {
998+
case nullValue:
999+
case intValue:
1000+
case uintValue:
1001+
case realValue:
1002+
case booleanValue:
1003+
value_ = other.value_;
1004+
break;
1005+
case stringValue:
1006+
if (other.value_.string_ && other.allocated_) {
1007+
unsigned len;
1008+
char const* str;
1009+
decodePrefixedString(other.allocated_, other.value_.string_,
1010+
&len, &str);
1011+
value_.string_ = duplicateAndPrefixStringValue(str, len);
1012+
allocated_ = true;
1013+
} else {
1014+
value_.string_ = other.value_.string_;
1015+
}
1016+
break;
1017+
case arrayValue:
1018+
case objectValue:
1019+
value_.map_ = new ObjectValues(*other.value_.map_);
1020+
break;
1021+
default:
1022+
JSON_ASSERT_UNREACHABLE;
1023+
}
1024+
}
1025+
10361026
void Value::releasePayload() {
10371027
switch (type_) {
10381028
case nullValue:
@@ -1054,6 +1044,22 @@ void Value::releasePayload() {
10541044
}
10551045
}
10561046

1047+
void Value::dupMeta(const Value& other) {
1048+
if (other.comments_) {
1049+
comments_ = new CommentInfo[numberOfCommentPlacement];
1050+
for (int comment = 0; comment < numberOfCommentPlacement; ++comment) {
1051+
const CommentInfo& otherComment = other.comments_[comment];
1052+
if (otherComment.comment_)
1053+
comments_[comment].setComment(
1054+
otherComment.comment_, strlen(otherComment.comment_));
1055+
}
1056+
} else {
1057+
comments_ = 0;
1058+
}
1059+
start_ = other.start_;
1060+
limit_ = other.limit_;
1061+
}
1062+
10571063
// Access an object value by name, create a null member if it does not exist.
10581064
// @pre Type of '*this' is object or null.
10591065
// @param key is null-terminated.

0 commit comments

Comments
 (0)