GenericValue: add copy constructor and CopyFrom #20
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
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. The explicit comparison against '0' is done to suppress the warning C4800 on MSVC, see pah/rapidjson#5.
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.
miloyip
added a commit
that referenced
this pull request
Jun 26, 2014
GenericValue: add copy constructor and CopyFrom
pah
added a commit
to pah/rapidjson-upstream
that referenced
this pull request
Jun 27, 2014
Another instance of casting away constness via C-style cast has been missed (introduced by Tencent#20).
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.(resubmitted #16 against
master)