Fix node_handle bugs:

-> Bad allocator destruction in swap
-> Wrong assertion in operator=
-> Make dangerous functions private

Added full testsuite.
This commit is contained in:
Ion Gaztañaga
2017-04-02 00:20:38 +02:00
parent e919196b8c
commit 01d7c71ea8
7 changed files with 793 additions and 116 deletions

View File

@@ -0,0 +1,55 @@
//////////////////////////////////////////////////////////////////////////////
//
// (C) Copyright Ion Gaztanaga 2005-2013. Distributed under the Boost
// Software License, Version 1.0. (See accompanying file
// LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
//
// See http://www.boost.org/libs/container for documentation.
//
//////////////////////////////////////////////////////////////////////////////
#ifndef BOOST_CONTAINER_PAIR_KEY_MAPPED_OF_VALUE_HPP
#define BOOST_CONTAINER_PAIR_KEY_MAPPED_OF_VALUE_HPP
#ifndef BOOST_CONFIG_HPP
# include <boost/config.hpp>
#endif
#if defined(BOOST_HAS_PRAGMA_ONCE)
# pragma once
#endif
#include <boost/container/detail/config_begin.hpp>
#include <boost/container/detail/workaround.hpp>
namespace boost {
namespace container {
template<class Key, class Mapped>
struct pair_key_mapped_of_value
{
typedef Key key_type;
typedef Mapped mapped_type;
template<class Pair>
const key_type & key_of_value(const Pair &p) const
{ return p.first; }
template<class Pair>
const mapped_type & mapped_of_value(const Pair &p) const
{ return p.second; }
template<class Pair>
key_type & key_of_value(Pair &p) const
{ return const_cast<key_type&>(p.first); }
template<class Pair>
mapped_type & mapped_of_value(Pair &p) const
{ return p.second; }
};
}}
#include <boost/container/detail/config_end.hpp>
#endif // BOOST_CONTAINER_PAIR_KEY_MAPPED_OF_VALUE_HPP

View File

@@ -493,7 +493,7 @@ class tree
typedef boost::container::reverse_iterator
<const_iterator> const_reverse_iterator;
typedef node_handle
< Node, value_type, allocator_type, void> node_type;
< NodeAlloc, void> node_type;
typedef insert_return_type_base
<iterator, node_type> insert_return_type;
@@ -1142,7 +1142,7 @@ class tree
this->insert_unique_check(hint, KeyOfValue()(nh.value()), data);
if(ret.second){
irt.inserted = true;
irt.position = iterator(this->icont().insert_unique_commit(*nh.get_node_pointer(), data));
irt.position = iterator(this->icont().insert_unique_commit(*nh.get(), data));
nh.release();
}
else{

View File

@@ -31,6 +31,8 @@
#include <boost/container/detail/type_traits.hpp>
#include <boost/container/detail/value_init.hpp>
#include <boost/container/detail/pair.hpp>
#include <boost/container/detail/pair_key_mapped_of_value.hpp>
// move
#include <boost/move/traits.hpp>
#include <boost/move/utility_core.hpp>
@@ -53,34 +55,6 @@
namespace boost {
namespace container {
///@cond
template<class Key, class Mapped>
struct pair_key_mapped_of_value
{
typedef Key key_type;
typedef Mapped mapped_type;
template<class Pair>
const key_type & key_of_value(const Pair &p) const
{ return p.first; }
template<class Pair>
const mapped_type & mapped_of_value(const Pair &p) const
{ return p.second; }
template<class Pair>
key_type & key_of_value(Pair &p) const
{ return const_cast<key_type&>(p.first); }
template<class Pair>
mapped_type & mapped_of_value(Pair &p) const
{ return p.second; }
};
///@endcond
#ifdef BOOST_CONTAINER_DOXYGEN_INVOKED
//! A map is a kind of associative container that supports unique keys (contains at
@@ -150,9 +124,7 @@ class map
typedef std::pair<key_type, mapped_type> nonconst_value_type;
typedef BOOST_CONTAINER_IMPDEF(movable_value_type_impl) movable_value_type;
typedef BOOST_CONTAINER_IMPDEF(node_handle<
typename base_t::node_type::container_node_type
BOOST_MOVE_I value_type
BOOST_MOVE_I allocator_type
typename base_t::stored_allocator_type
BOOST_MOVE_I pair_key_mapped_of_value
<key_type BOOST_MOVE_I mapped_type> >) node_type;
typedef BOOST_CONTAINER_IMPDEF
@@ -1243,9 +1215,7 @@ class multimap
typedef std::pair<key_type, mapped_type> nonconst_value_type;
typedef BOOST_CONTAINER_IMPDEF(movable_value_type_impl) movable_value_type;
typedef BOOST_CONTAINER_IMPDEF(node_handle<
typename base_t::node_type::container_node_type
BOOST_MOVE_I value_type
BOOST_MOVE_I allocator_type
typename base_t::stored_allocator_type
BOOST_MOVE_I pair_key_mapped_of_value
<key_type BOOST_MOVE_I mapped_type> >) node_type;

View File

@@ -40,20 +40,34 @@ namespace container {
///@cond
template<class Value, class KeyMapped, bool keymapped_is_void = container_detail::is_same<KeyMapped, void>::value>
template<class Value, class KeyMapped>
struct node_handle_keymapped_traits
{
typedef typename KeyMapped::key_type key_type;
typedef typename KeyMapped::mapped_type mapped_type;
};
template<class Value>
struct node_handle_keymapped_traits<Value, void>
{
typedef Value key_type;
typedef Value mapped_type;
};
template<class Value, class KeyMapped>
struct node_handle_keymapped_traits<Value, KeyMapped, false>
class node_handle_friend
{
typedef typename KeyMapped::key_type key_type;
typedef typename KeyMapped::mapped_type mapped_type;
public:
template<class NH>
BOOST_CONTAINER_FORCEINLINE static void destroy_alloc(NH &nh) BOOST_NOEXCEPT
{ nh.destroy_alloc(); }
template<class NH>
BOOST_CONTAINER_FORCEINLINE static typename NH::node_pointer &get_node_pointer(NH &nh) BOOST_NOEXCEPT
{ return nh.get_node_pointer(); }
};
///@endcond
//! A node_handle is an object that accepts ownership of a single element from an associative container.
@@ -73,76 +87,63 @@ struct node_handle_keymapped_traits<Value, KeyMapped, false>
//!
//! If a node handle is not empty, then it contains an allocator that is equal to the allocator of the container
//! when the element was extracted. If a node handle is empty, it contains no allocator.
template <class NodeType, class Value, class Allocator, class KeyMapped = void>
template <class NodeAllocator, class KeyMapped = void>
class node_handle
{
typedef node_handle_keymapped_traits<Value, KeyMapped> keymapped_t;
typedef NodeAllocator nallocator_type;
typedef allocator_traits<NodeAllocator> nator_traits;
typedef typename nator_traits::value_type priv_node_t;
typedef typename priv_node_t::value_type priv_value_t;
typedef node_handle_keymapped_traits<priv_value_t, KeyMapped> keymapped_t;
public:
typedef Value value_type;
typedef priv_value_t value_type;
typedef typename keymapped_t::key_type key_type;
typedef typename keymapped_t::mapped_type mapped_type;
typedef Allocator allocator_type;
typedef NodeType container_node_type;
typedef typename nator_traits::template portable_rebind_alloc
<value_type>::type allocator_type;
typedef priv_node_t container_node_type;
friend class node_handle_friend;
///@cond
private:
BOOST_MOVABLE_BUT_NOT_COPYABLE(node_handle)
typedef allocator_traits<allocator_type> ator_traits;
typedef typename ator_traits::template portable_rebind_alloc
<container_node_type>::type nallocator_type;
typedef allocator_traits<nallocator_type> node_ator_traits;
typedef typename node_ator_traits::pointer node_pointer;
typedef typename nator_traits::pointer node_pointer;
typedef ::boost::aligned_storage
<sizeof(allocator_type), boost::alignment_of<nallocator_type>::value> nalloc_storage_t;
< sizeof(nallocator_type)
, boost::alignment_of<nallocator_type>::value> nalloc_storage_t;
node_pointer m_ptr;
nalloc_storage_t m_nalloc_storage;
void move_construct_alloc(nallocator_type &al)
{ ::new(m_nalloc_storage.address(), boost_container_new_t()) allocator_type(::boost::move(al)); }
{ ::new(m_nalloc_storage.address(), boost_container_new_t()) nallocator_type(::boost::move(al)); }
void destroy_node()
void destroy_deallocate_node()
{
node_ator_traits::destroy(this->node_alloc(), container_detail::to_raw_pointer(m_ptr));
node_ator_traits::deallocate(this->node_alloc(), m_ptr, 1u);
nator_traits::destroy(this->node_alloc(), container_detail::to_raw_pointer(m_ptr));
nator_traits::deallocate(this->node_alloc(), m_ptr, 1u);
}
template<class OtherNodeHandle>
void move_construct_end(OtherNodeHandle &nh)
{
if(m_ptr){
::new (m_nalloc_storage.address(), boost_container_new_t()) allocator_type(::boost::move(nh.node_alloc()));
nh.destroy_alloc();
nh.get_node_pointer() = node_pointer();
::new (m_nalloc_storage.address(), boost_container_new_t()) nallocator_type(::boost::move(nh.node_alloc()));
node_handle_friend::destroy_alloc(nh);
node_handle_friend::get_node_pointer(nh) = node_pointer();
}
BOOST_ASSERT(nh.empty());
}
public:
void destroy_alloc() BOOST_NOEXCEPT
{ static_cast<nallocator_type*>(m_nalloc_storage.address())->~nallocator_type(); }
void destroy_alloc()
{ static_cast<allocator_type*>(m_nalloc_storage.address())->~allocator_type(); }
node_pointer &get_node_pointer()
node_pointer &get_node_pointer() BOOST_NOEXCEPT
{ return m_ptr; }
nallocator_type &node_alloc()
{ return *static_cast<nallocator_type*>(m_nalloc_storage.address()); }
const nallocator_type &node_alloc() const
{ return *static_cast<const nallocator_type*>(m_nalloc_storage.address()); }
node_pointer release()
{
node_pointer p(m_ptr);
m_ptr = node_pointer();
if(p)
this->destroy_alloc();
return p;
}
///@endcond
public:
@@ -150,13 +151,13 @@ class node_handle
//!
//! <b>Postcondition</b>: this->empty()
BOOST_CXX14_CONSTEXPR node_handle() BOOST_NOEXCEPT
: m_ptr(), m_nalloc_storage()
{ BOOST_ASSERT(this->empty()); }
: m_ptr()
{ }
//! <b>Effects</b>: Constructs a node_handle object initializing internal pointer with p.
//! If p != nullptr copy constructs internal allocator al.
//! If p != nullptr copy constructs internal allocator from al.
node_handle(node_pointer p, const nallocator_type &al) BOOST_NOEXCEPT
: m_ptr(p), m_nalloc_storage()
: m_ptr(p)
{
if(m_ptr){
::new (m_nalloc_storage.address(), boost_container_new_t()) nallocator_type(al);
@@ -172,12 +173,12 @@ class node_handle
//! <b>Note</b>: Two node_handle's are related if only one of KeyMapped template parameter
//! of a node handle is void.
template<class KeyMapped2>
node_handle( BOOST_RV_REF_BEG node_handle<NodeType, Value, Allocator, KeyMapped2> BOOST_RV_REF_END nh
node_handle( BOOST_RV_REF_BEG node_handle<NodeAllocator, KeyMapped2> BOOST_RV_REF_END nh
, typename container_detail::enable_if_c
< ((unsigned)container_detail::is_same<KeyMapped, void>::value +
(unsigned)container_detail::is_same<KeyMapped2, void>::value) == 1u
>::type* = 0)
: m_ptr(nh.get_node_pointer()), m_nalloc_storage()
>::type* = 0) BOOST_NOEXCEPT
: m_ptr(nh.get())
{ this->move_construct_end(nh); }
//! <b>Effects</b>: Constructs a node_handle object initializing internal pointer with nh's internal pointer
@@ -186,43 +187,44 @@ class node_handle
//!
//! <b>Postcondition</b>: nh.empty()
node_handle (BOOST_RV_REF(node_handle) nh) BOOST_NOEXCEPT
: m_ptr(nh.m_ptr), m_nalloc_storage()
: m_ptr(nh.m_ptr)
{ this->move_construct_end(nh); }
//! <b>Effects</b>: If !this->empty(), destroys the value_type subobject in the container_node_type object
//! pointed to by c by calling allocator_traits<impl_defined>::destroy, then deallocates m_ptr by calling
//! ator_traits::rebind_traits<container_node_type>::deallocate.
~node_handle () BOOST_NOEXCEPT
//! nator_traits::rebind_traits<container_node_type>::deallocate.
~node_handle() BOOST_NOEXCEPT
{
if(!this->empty()){
this->destroy_node();
this->destroy_deallocate_node();
this->destroy_alloc();
}
}
//! <b>Requires</b>: Either this->empty(), or ator_traits::propagate_on_container_move_assignment is true, or
//! <b>Requires</b>: Either this->empty(), or nator_traits::propagate_on_container_move_assignment is true, or
//! node_alloc() == nh.node_alloc().
//!
//! <b>Effects</b>: If m_ptr != nullptr, destroys the value_type subobject in the container_node_type object
//! pointed to by m_ptr by calling ator_traits::destroy, then deallocates m_ptr by calling ator_-
//! traits::rebind_traits<container_node_type>::deallocate. Assigns nh.m_ptr to m_ptr. If this->empty()
//! or ator_traits::propagate_on_container_move_assignment is true, move assigns nh.node_alloc() to
//! pointed to by m_ptr by calling nator_traits::destroy, then deallocates m_ptr by calling
//! nator_traits::deallocate. Assigns nh.m_ptr to m_ptr. If this->empty()
//! or nator_traits::propagate_on_container_move_assignment is true, move assigns nh.node_alloc() to
//! node_alloc(). Assigns nullptr to nh.m_ptr and assigns nullopt to nh.node_alloc().
//!
//! <b>Returns</b>: *this.
//!
//! <b>Throws</b>: Nothing.
node_handle & operator=(BOOST_RV_REF(node_handle) nh)
node_handle & operator=(BOOST_RV_REF(node_handle) nh) BOOST_NOEXCEPT
{
BOOST_ASSERT(this->empty() || nh.empty() || ator_traits::propagate_on_container_move_assignment::value
|| ator_traits::equal(node_alloc(), nh.node_alloc()));
BOOST_ASSERT(this->empty() || nator_traits::propagate_on_container_move_assignment::value
|| nator_traits::equal(node_alloc(), nh.node_alloc()));
bool const was_this_non_null = !this->empty();
bool const was_nh_non_null = !nh.empty();
if(was_nh_non_null){
if(was_this_non_null){
this->destroy_node();
if(ator_traits::propagate_on_container_move_assignment::value){
this->destroy_deallocate_node();
if(nator_traits::propagate_on_container_move_assignment::value){
this->node_alloc() = ::boost::move(nh.node_alloc());
}
}
@@ -234,7 +236,7 @@ class node_handle
nh.destroy_alloc();
}
else if(was_this_non_null){
this->destroy_node();
this->destroy_deallocate_node();
this->destroy_alloc();
m_ptr = node_pointer();
}
@@ -310,23 +312,23 @@ class node_handle
return !this->m_ptr;
}
//! <b>Requires</b>: this->empty(), or nh.empty(), or ator_traits::propagate_on_container_swap is true, or
//! <b>Requires</b>: this->empty(), or nh.empty(), or nator_traits::propagate_on_container_swap is true, or
//! node_alloc() == nh.node_alloc().
//!
//! <b>Effects</b>: Calls swap(m_ptr, nh.m_ptr). If this->empty(), or nh.empty(), or ator_traits::propagate_on_-
//! <b>Effects</b>: Calls swap(m_ptr, nh.m_ptr). If this->empty(), or nh.empty(), or nator_traits::propagate_on_-
//! container_swap is true calls swap(node_alloc(), nh.node_alloc()).
void swap(node_handle &nh)
BOOST_NOEXCEPT_IF(ator_traits::propagate_on_container_swap::value || ator_traits::is_always_equal::value)
BOOST_NOEXCEPT_IF(nator_traits::propagate_on_container_swap::value || nator_traits::is_always_equal::value)
{
BOOST_ASSERT(this->empty() || nh.empty() || ator_traits::propagate_on_container_swap::value
|| ator_traits::equal(node_alloc(), nh.node_alloc()));
BOOST_ASSERT(this->empty() || nh.empty() || nator_traits::propagate_on_container_swap::value
|| nator_traits::equal(node_alloc(), nh.node_alloc()));
bool const was_this_non_null = !this->empty();
bool const was_nh_non_null = !nh.empty();
if(was_nh_non_null){
if(was_this_non_null){
if(ator_traits::propagate_on_container_swap::value){
if(nator_traits::propagate_on_container_swap::value){
::boost::adl_move_swap(this->node_alloc(), nh.node_alloc());
}
}
@@ -337,11 +339,53 @@ class node_handle
}
else if(was_this_non_null){
nh.move_construct_alloc(this->node_alloc());
nh.destroy_alloc();
this->destroy_alloc();
}
::boost::adl_move_swap(m_ptr, nh.m_ptr);
}
//! <b>Effects</b>: If this->empty() returns nullptr, otherwise returns m_ptr
//! resets m_ptr to nullptr and destroys the internal allocator.
//!
//! <b>Postcondition</b>: this->empty()
//!
//! <b>Note</b>: Non-standard extensions
node_pointer release() BOOST_NOEXCEPT
{
node_pointer p(m_ptr);
m_ptr = node_pointer();
if(p)
this->destroy_alloc();
return p;
}
//! <b>Effects</b>: Returns m_ptr.
//!
//! <b>Note</b>: Non-standard extensions
node_pointer get() const BOOST_NOEXCEPT
{
return m_ptr;
}
//! <b>Effects</b>: Returns a reference to the internal node allocator.
//!
//! <b>Note</b>: Non-standard extensions
nallocator_type &node_alloc() BOOST_NOEXCEPT
{
BOOST_ASSERT(!empty());
return *static_cast<nallocator_type*>(m_nalloc_storage.address());
}
//! <b>Effects</b>: Returns a reference to the internal node allocator.
//!
//! <b>Note</b>: Non-standard extensions
const nallocator_type &node_alloc() const BOOST_NOEXCEPT
{
BOOST_ASSERT(!empty());
return *static_cast<const nallocator_type*>(m_nalloc_storage.address());
}
//! <b>Effects</b>: x.swap(y).
//!
friend void swap(node_handle & x, node_handle & y) BOOST_NOEXCEPT_IF(BOOST_NOEXCEPT(x.swap(y)))