GenericValue: add copy constructor and CopyFrom #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 31, 2014
Closed
Instead of always just shallowly referencing the potentially allocated strings when calling the Handler::String function, request a copy in case the string has been allocated from an Allocator before. This is necessary to avoid double free()s of the string memory, especially when using the Handler to create a deep copy of a Value.
To allow deep copying from an existing GenericValue, an
explicit "copy constructor" (with required Allocator param)
and an "CopyFrom" assignment function are added.
Document d; Document::AllocatorType& a = d.GetAllocator();
Value v1("foo");
// Value v2(v1); // not allowed
Value v2(v1,a); // make a copy
RAPIDJSON_ASSERT(v1.IsString()); // v1 untouched
d.SetArray().PushBack(v1,a).PushBack(v2,a);
RAPIDJSON_ASSERT(v1.Empty() && v2.Empty());
v2.CopyFrom(d,a); // copy whole document
RAPIDJSON_ASSERT(d.IsArray() && d.Size()); // d untouched
v1.SetObject().AddMember( "array", v2, a );
d.PushBack(v1,a);
Additionally, the Handler implementation in GenericDocument is made
private again, restricting access to GenericReader and GenericValue.
This commit adds some simple tests for the deep-copying of values, either based on the explicit constructor, or the CopyFrom function. It uses the CrtAllocator to test for possible double-free errors due to insufficient copying.
|
Unfortunately, GitHub doesn't allow to "re-open" a pull-request and accidentally this PR has been marked to be against |
|
See upstream issue 12 for a related discussion. |
|
Updated |
|
See upstream pull-request Tencent/rapidjson#20. |
|
Merged upstream. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To allow deep copying from an existing
GenericValue, an explicit "copy constructor" (with required Allocator param) and aCopyFromassignment function are added.The strings in the source value are copied, if and only if they have been allocated as a copy during their construction (determined by
kCopyFlag). This is needed to avoid doublefree()s or problems with differing lifetimes of the allocated string storage.Additionally, the
Handlerimplementation inGenericDocumentis made private again, restricting access toGenericReaderandGenericValue.