XPath: Refactor xpath_node_set short buffer optimization

This change replaces xpath_node_set single element storage with a
single-element array in hopes that this would silence Coverity false
positive about getting a singleton pointer.

Additionally, it refactors _assign member to unify small and large
buffer codepaths since they are basically identical.

Fixes #233 (hopefully)
This commit is contained in:
Arseny Kapoulkine 2018-09-24 20:18:12 -07:00
parent 1a96777b11
commit e3b5e9ce3c
2 changed files with 28 additions and 41 deletions

View File

@ -12013,74 +12013,61 @@ namespace pugi
size_t size_ = static_cast<size_t>(end_ - begin_); size_t size_ = static_cast<size_t>(end_ - begin_);
if (size_ <= 1) // use internal buffer for 0 or 1 elements, heap buffer otherwise
xpath_node* storage = (size_ <= 1) ? _storage : static_cast<xpath_node*>(impl::xml_memory::allocate(size_ * sizeof(xpath_node)));
if (!storage)
{ {
// deallocate old buffer #ifdef PUGIXML_NO_EXCEPTIONS
if (_begin != &_storage) impl::xml_memory::deallocate(_begin); return;
#else
// use internal buffer throw std::bad_alloc();
if (begin_ != end_) _storage = *begin_; #endif
_begin = &_storage;
_end = &_storage + size_;
_type = type_;
} }
else
{
// make heap copy
xpath_node* storage = static_cast<xpath_node*>(impl::xml_memory::allocate(size_ * sizeof(xpath_node)));
if (!storage) // deallocate old buffer
{ if (_begin != _storage)
#ifdef PUGIXML_NO_EXCEPTIONS impl::xml_memory::deallocate(_begin);
return;
#else
throw std::bad_alloc();
#endif
}
// size check is necessary because for begin_ = end_ = nullptr, memcpy is UB
if (size_)
memcpy(storage, begin_, size_ * sizeof(xpath_node)); memcpy(storage, begin_, size_ * sizeof(xpath_node));
// deallocate old buffer _begin = storage;
if (_begin != &_storage) impl::xml_memory::deallocate(_begin); _end = storage + size_;
_type = type_;
// finalize
_begin = storage;
_end = storage + size_;
_type = type_;
}
} }
#ifdef PUGIXML_HAS_MOVE #ifdef PUGIXML_HAS_MOVE
PUGI__FN void xpath_node_set::_move(xpath_node_set& rhs) PUGIXML_NOEXCEPT PUGI__FN void xpath_node_set::_move(xpath_node_set& rhs) PUGIXML_NOEXCEPT
{ {
_type = rhs._type; _type = rhs._type;
_storage = rhs._storage; _storage[0] = rhs._storage[0];
_begin = (rhs._begin == &rhs._storage) ? &_storage : rhs._begin; _begin = (rhs._begin == rhs._storage) ? _storage : rhs._begin;
_end = _begin + (rhs._end - rhs._begin); _end = _begin + (rhs._end - rhs._begin);
rhs._type = type_unsorted; rhs._type = type_unsorted;
rhs._begin = &rhs._storage; rhs._begin = rhs._storage;
rhs._end = rhs._begin; rhs._end = rhs._storage;
} }
#endif #endif
PUGI__FN xpath_node_set::xpath_node_set(): _type(type_unsorted), _begin(&_storage), _end(&_storage) PUGI__FN xpath_node_set::xpath_node_set(): _type(type_unsorted), _begin(_storage), _end(_storage)
{ {
} }
PUGI__FN xpath_node_set::xpath_node_set(const_iterator begin_, const_iterator end_, type_t type_): _type(type_unsorted), _begin(&_storage), _end(&_storage) PUGI__FN xpath_node_set::xpath_node_set(const_iterator begin_, const_iterator end_, type_t type_): _type(type_unsorted), _begin(_storage), _end(_storage)
{ {
_assign(begin_, end_, type_); _assign(begin_, end_, type_);
} }
PUGI__FN xpath_node_set::~xpath_node_set() PUGI__FN xpath_node_set::~xpath_node_set()
{ {
if (_begin != &_storage) if (_begin != _storage)
impl::xml_memory::deallocate(_begin); impl::xml_memory::deallocate(_begin);
} }
PUGI__FN xpath_node_set::xpath_node_set(const xpath_node_set& ns): _type(type_unsorted), _begin(&_storage), _end(&_storage) PUGI__FN xpath_node_set::xpath_node_set(const xpath_node_set& ns): _type(type_unsorted), _begin(_storage), _end(_storage)
{ {
_assign(ns._begin, ns._end, ns._type); _assign(ns._begin, ns._end, ns._type);
} }
@ -12095,7 +12082,7 @@ namespace pugi
} }
#ifdef PUGIXML_HAS_MOVE #ifdef PUGIXML_HAS_MOVE
PUGI__FN xpath_node_set::xpath_node_set(xpath_node_set&& rhs) PUGIXML_NOEXCEPT: _type(type_unsorted), _begin(&_storage), _end(&_storage) PUGI__FN xpath_node_set::xpath_node_set(xpath_node_set&& rhs) PUGIXML_NOEXCEPT: _type(type_unsorted), _begin(_storage), _end(_storage)
{ {
_move(rhs); _move(rhs);
} }
@ -12104,7 +12091,7 @@ namespace pugi
{ {
if (this == &rhs) return *this; if (this == &rhs) return *this;
if (_begin != &_storage) if (_begin != _storage)
impl::xml_memory::deallocate(_begin); impl::xml_memory::deallocate(_begin);
_move(rhs); _move(rhs);

View File

@ -1372,7 +1372,7 @@ namespace pugi
private: private:
type_t _type; type_t _type;
xpath_node _storage; xpath_node _storage[1];
xpath_node* _begin; xpath_node* _begin;
xpath_node* _end; xpath_node* _end;