Skip to content

Commit 0ced843

Browse files
cdunn2001cdunn2001
authored
Merge pull request open-source-parsers#726 from okodron/fix-704
Value::copy() creates a deep copy now
2 parents 2f227cb + 9b569c8 commit 0ced843

3 files changed

Lines changed: 115 additions & 66 deletions

File tree

‎include/json/value.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +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);
610+
void releasePayload();
611+
void dupMeta(const Value& other);
609612

610613
Value& resolveReference(const char* key);
611614
Value& resolveReference(const char* key, const char* end);

‎src/lib_json/json_value.cpp‎

Lines changed: 77 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -441,48 +441,9 @@ Value::Value(bool value) {
441441
value_.bool_ = value;
442442
}
443443

444-
Value::Value(Value const& other)
445-
: type_(other.type_), allocated_(false)
446-
,
447-
comments_(0), start_(other.start_), limit_(other.limit_)
448-
{
449-
switch (type_) {
450-
case nullValue:
451-
case intValue:
452-
case uintValue:
453-
case realValue:
454-
case booleanValue:
455-
value_ = other.value_;
456-
break;
457-
case stringValue:
458-
if (other.value_.string_ && other.allocated_) {
459-
unsigned len;
460-
char const* str;
461-
decodePrefixedString(other.allocated_, other.value_.string_,
462-
&len, &str);
463-
value_.string_ = duplicateAndPrefixStringValue(str, len);
464-
allocated_ = true;
465-
} else {
466-
value_.string_ = other.value_.string_;
467-
allocated_ = false;
468-
}
469-
break;
470-
case arrayValue:
471-
case objectValue:
472-
value_.map_ = new ObjectValues(*other.value_.map_);
473-
break;
474-
default:
475-
JSON_ASSERT_UNREACHABLE;
476-
}
477-
if (other.comments_) {
478-
comments_ = new CommentInfo[numberOfCommentPlacement];
479-
for (int comment = 0; comment < numberOfCommentPlacement; ++comment) {
480-
const CommentInfo& otherComment = other.comments_[comment];
481-
if (otherComment.comment_)
482-
comments_[comment].setComment(
483-
otherComment.comment_, strlen(otherComment.comment_));
484-
}
485-
}
444+
Value::Value(const Value& other) {
445+
dupPayload(other);
446+
dupMeta(other);
486447
}
487448

488449
#if JSON_HAS_RVALUE_REFERENCES
@@ -494,24 +455,7 @@ Value::Value(Value&& other) {
494455
#endif
495456

496457
Value::~Value() {
497-
switch (type_) {
498-
case nullValue:
499-
case intValue:
500-
case uintValue:
501-
case realValue:
502-
case booleanValue:
503-
break;
504-
case stringValue:
505-
if (allocated_)
506-
releasePrefixedStringValue(value_.string_);
507-
break;
508-
case arrayValue:
509-
case objectValue:
510-
delete value_.map_;
511-
break;
512-
default:
513-
JSON_ASSERT_UNREACHABLE;
514-
}
458+
releasePayload();
515459

516460
delete[] comments_;
517461

@@ -534,9 +478,8 @@ void Value::swapPayload(Value& other) {
534478
}
535479

536480
void Value::copyPayload(const Value& other) {
537-
type_ = other.type_;
538-
value_ = other.value_;
539-
allocated_ = other.allocated_;
481+
releasePayload();
482+
dupPayload(other);
540483
}
541484

542485
void Value::swap(Value& other) {
@@ -548,9 +491,8 @@ void Value::swap(Value& other) {
548491

549492
void Value::copy(const Value& other) {
550493
copyPayload(other);
551-
comments_ = other.comments_;
552-
start_ = other.start_;
553-
limit_ = other.limit_;
494+
delete[] comments_;
495+
dupMeta(other);
554496
}
555497

556498
ValueType Value::type() const { return type_; }
@@ -1049,6 +991,75 @@ void Value::initBasic(ValueType vtype, bool allocated) {
1049991
limit_ = 0;
1050992
}
1051993

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+
1026+
void Value::releasePayload() {
1027+
switch (type_) {
1028+
case nullValue:
1029+
case intValue:
1030+
case uintValue:
1031+
case realValue:
1032+
case booleanValue:
1033+
break;
1034+
case stringValue:
1035+
if (allocated_)
1036+
releasePrefixedStringValue(value_.string_);
1037+
break;
1038+
case arrayValue:
1039+
case objectValue:
1040+
delete value_.map_;
1041+
break;
1042+
default:
1043+
JSON_ASSERT_UNREACHABLE;
1044+
}
1045+
}
1046+
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+
10521063
// Access an object value by name, create a null member if it does not exist.
10531064
// @pre Type of '*this' is object or null.
10541065
// @param key is null-terminated.

‎src/test_lib_json/main.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,6 +1433,40 @@ JSONTEST_FIXTURE(ValueTest, compareType) {
14331433
Json::Value(Json::objectValue)));
14341434
}
14351435

1436+
JSONTEST_FIXTURE(ValueTest, CopyObject) {
1437+
Json::Value arrayVal;
1438+
arrayVal.append("val1");
1439+
arrayVal.append("val2");
1440+
arrayVal.append("val3");
1441+
Json::Value stringVal("string value");
1442+
Json::Value copy1, copy2;
1443+
{
1444+
Json::Value arrayCopy, stringCopy;
1445+
arrayCopy.copy(arrayVal);
1446+
stringCopy.copy(stringVal);
1447+
JSONTEST_ASSERT_PRED(checkIsEqual(arrayCopy, arrayVal));
1448+
JSONTEST_ASSERT_PRED(checkIsEqual(stringCopy, stringVal));
1449+
arrayCopy.append("val4");
1450+
JSONTEST_ASSERT(arrayCopy.size() == 4);
1451+
arrayVal.append("new4");
1452+
arrayVal.append("new5");
1453+
JSONTEST_ASSERT(arrayVal.size() == 5);
1454+
JSONTEST_ASSERT(!(arrayCopy == arrayVal));
1455+
stringCopy = "another string";
1456+
JSONTEST_ASSERT(!(stringCopy == stringVal));
1457+
copy1.copy(arrayCopy);
1458+
copy2.copy(stringCopy);
1459+
}
1460+
JSONTEST_ASSERT(arrayVal.size() == 5);
1461+
JSONTEST_ASSERT(stringVal == "string value");
1462+
JSONTEST_ASSERT(copy1.size() == 4);
1463+
JSONTEST_ASSERT(copy2 == "another string");
1464+
copy1.copy(stringVal);
1465+
JSONTEST_ASSERT(copy1 == "string value");
1466+
copy2.copy(arrayVal);
1467+
JSONTEST_ASSERT(copy2.size() == 5);
1468+
}
1469+
14361470
void ValueTest::checkIsLess(const Json::Value& x, const Json::Value& y) {
14371471
JSONTEST_ASSERT(x < y);
14381472
JSONTEST_ASSERT(y > x);
@@ -2544,6 +2578,7 @@ int main(int argc, const char* argv[]) {
25442578
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, compareArray);
25452579
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, compareObject);
25462580
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, compareType);
2581+
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, CopyObject);
25472582
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, offsetAccessors);
25482583
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, typeChecksThrowExceptions);
25492584
JSONTEST_REGISTER_FIXTURE(runner, ValueTest, StaticString);

0 commit comments

Comments
 (0)