Skip to content

Commit

Permalink
Avoid calling memcpy with NULL pointers
Browse files Browse the repository at this point in the history
According to the C/C++ standards, calling `memcpy(NULL, NULL, 0)` is
undefined behaviour. Recent GCC versions may rely on this by optimizing
NULL pointer checks more aggressively, see [1].

This patch tries to avoid calling std::memcpy with zero elements.
As a side effect, explicitly return NULL when requesting an empty block
from MemoryPoolAllocator::Malloc.

This may be related to #301.

[1] https://gcc.gnu.org/gcc-4.9/porting_to.html
  • Loading branch information
pah committed Apr 16, 2015
1 parent 94c0082 commit 0c5c153
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
7 changes: 6 additions & 1 deletion include/rapidjson/allocators.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ class MemoryPoolAllocator {

//! Allocates a memory block. (concept Allocator)
void* Malloc(size_t size) {
if (!size)
return NULL;

size = RAPIDJSON_ALIGN(size);
if (chunkHead_ == 0 || chunkHead_->size + size > chunkHead_->capacity)
AddChunk(chunk_capacity_ > size ? chunk_capacity_ : size);
Expand Down Expand Up @@ -191,7 +194,9 @@ class MemoryPoolAllocator {
// Realloc process: allocate and copy memory, do not free original buffer.
void* newBuffer = Malloc(newSize);
RAPIDJSON_ASSERT(newBuffer != 0); // Do not handle out-of-memory explicitly.
return std::memcpy(newBuffer, originalPtr, originalSize);
if (originalSize)
std::memcpy(newBuffer, originalPtr, originalSize);
return newBuffer;
}

//! Frees a memory block (concept Allocator)
Expand Down
16 changes: 12 additions & 4 deletions include/rapidjson/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1582,16 +1582,24 @@ class GenericValue {
// Initialize this value as array with initial data, without calling destructor.
void SetArrayRaw(GenericValue* values, SizeType count, Allocator& allocator) {
flags_ = kArrayFlag;
data_.a.elements = (GenericValue*)allocator.Malloc(count * sizeof(GenericValue));
std::memcpy(data_.a.elements, values, count * sizeof(GenericValue));
if (count) {
data_.a.elements = (GenericValue*)allocator.Malloc(count * sizeof(GenericValue));
std::memcpy(data_.a.elements, values, count * sizeof(GenericValue));
}
else
data_.a.elements = NULL;
data_.a.size = data_.a.capacity = count;
}

//! Initialize this value as object with initial data, without calling destructor.
void SetObjectRaw(Member* members, SizeType count, Allocator& allocator) {
flags_ = kObjectFlag;
data_.o.members = (Member*)allocator.Malloc(count * sizeof(Member));
std::memcpy(data_.o.members, members, count * sizeof(Member));
if (count) {
data_.o.members = (Member*)allocator.Malloc(count * sizeof(Member));
std::memcpy(data_.o.members, members, count * sizeof(Member));
}
else
data_.o.members = NULL;
data_.o.size = data_.o.capacity = count;
}

Expand Down

0 comments on commit 0c5c153

Please sign in to comment.