Fix basic_fields allocator awareness

CHANGELOG.md

@@ -3,6 +3,7 @@ Version 50
 * parser is constructible from other body types
 * Add field enumeration
 * Use allocator more in basic_fields
+* Fix basic_fields allocator awareness
 
 API Changes:
 

include/beast/http/fields.hpp

@@ -81,13 +81,13 @@ public:
     /// A constant iterator to the field sequence.
     using iterator = const_iterator;
 
-    /// Default constructor.
-    basic_fields() = default;
-
     /// Destructor
     ~basic_fields();
 
-    /** Construct the fields.
+    /// Default constructor.
+    basic_fields() = default;
+
+    /** Constructor.
 
         @param alloc The allocator to use.
     */
@@ -96,32 +96,60 @@ public:
 
     /** Move constructor.
 
-        The moved-from object becomes an empty field sequence.
+        The moved-from object behaves as if by call to @ref clear.
-
-        @param other The object to move from.
     */
-    basic_fields(basic_fields&& other);
+    basic_fields(basic_fields&&);
-
-    /** Move assignment.
-
-        The moved-from object becomes an empty field sequence.
-
-        @param other The object to move from.
-    */
-    basic_fields& operator=(basic_fields&& other);
 
     /// Copy constructor.
     basic_fields(basic_fields const&);
 
+    /** Copy constructor.
+
+        @param alloc The allocator to use.
+    */
+    basic_fields(basic_fields const&, Allocator const& alloc);
+
+    /// Copy constructor.
+#if BEAST_DOXYGEN
+    template<class OtherAlloc>
+#else
+    template<class OtherAlloc, class =
+        typename std::enable_if<! std::is_same<OtherAlloc,
+            Allocator>::value>::type>
+#endif
+    basic_fields(basic_fields<OtherAlloc> const&);
+
+    /** Copy constructor.
+
+        @param alloc The allocator to use.
+    */
+#if BEAST_DOXYGEN
+    template<class OtherAlloc>
+#else
+    template<class OtherAlloc, class =
+        typename std::enable_if<! std::is_same<OtherAlloc,
+            Allocator>::value>::type>
+#endif
+    basic_fields(basic_fields<OtherAlloc> const&,
+        Allocator const& alloc);
+
+    /** Move assignment.
+
+        The moved-from object behaves as if by call to @ref clear.
+    */
+    basic_fields& operator=(basic_fields&&);
+
     /// Copy assignment.
     basic_fields& operator=(basic_fields const&);
 
-    /// Copy constructor.
-    template<class OtherAlloc>
-    basic_fields(basic_fields<OtherAlloc> const&);
-
     /// Copy assignment.
+#if BEAST_DOXYGEN
     template<class OtherAlloc>
+#else
+    template<class OtherAlloc, class =
+        typename std::enable_if<! std::is_same<OtherAlloc,
+            Allocator>::value>::type>
+#endif
     basic_fields& operator=(basic_fields<OtherAlloc> const&);
 
     /// Return a copy of the allocator associated with the container.
@@ -269,6 +297,12 @@ public:
             boost::lexical_cast<std::string>(value));
     }
 
+    /// Swap two field containers
+    template<class Alloc>
+    friend
+    void
+    swap(basic_fields<Alloc>& lhs, basic_fields<Alloc>& rhs);
+
 protected:
     // for header
     string_view method_impl() const;
@@ -284,31 +318,25 @@ protected:
     void chunked_impl();
 
 private:
-    struct element
+    class element
-        : boost::intrusive::set_base_hook <
+        : public boost::intrusive::set_base_hook <
             boost::intrusive::link_mode <
                 boost::intrusive::normal_link>>
-        , boost::intrusive::list_base_hook <
+        , public boost::intrusive::list_base_hook <
             boost::intrusive::link_mode <
                 boost::intrusive::normal_link>>
     {
-        element(
+        off_t off_;
-            string_view name, string_view value)
+        off_t len_;
-        {
+
-            char* p = reinterpret_cast<char*>(this + 1);
+    public:
-            data.first = p;
+        element(string_view name, string_view value);
-            data.off =
+
-                static_cast<off_t>(name.size() + 2);
+        string_view
-            data.len =
+        name() const;
-                static_cast<off_t>(value.size());
+
-            std::memcpy(p, name.data(), name.size());
+        string_view
-            p[data.off-2] = ':';
+        value() const;
-            p[data.off-1] = ' ';
-            std::memcpy(
-                p + data.off, value.data(), value.size());
-            p[data.off + data.len] = '\r';
-            p[data.off + data.len + 1] = '\n';
-        }
 
         value_type data;
     };
@@ -355,37 +383,47 @@ private:
         element, boost::intrusive::constant_time_size<
             true>, boost::intrusive::compare<less>>::type;
 
-    void
-    delete_all();
-
-    void
-    move_assign(basic_fields&, std::false_type);
-
-    void
-    move_assign(basic_fields&, std::true_type);
-
-    void
-    copy_assign(basic_fields const&, std::false_type);
-
-    void
-    copy_assign(basic_fields const&, std::true_type);
-
-    template<class FieldSequence>
-    void
-    copy_from(FieldSequence const& fs)
-    {
-        for(auto const& e : fs)
-            insert(e.name(), e.value());
-    }
-
     element&
     new_element(string_view name, string_view value);
 
     void
     delete_element(element& e);
 
+    void
+    realloc_string(string_view& dest, string_view s);
+
+    template<class OtherAlloc>
+    void
+    copy_all(basic_fields<OtherAlloc> const&);
+
+    void
+    clear_all();
+
+    void
+    delete_list();
+
+    void
+    move_assign(basic_fields&, std::true_type);
+
+    void
+    move_assign(basic_fields&, std::false_type);
+
+    void
+    copy_assign(basic_fields const&, std::true_type);
+
+    void
+    copy_assign(basic_fields const&, std::false_type);
+
+    void
+    swap(basic_fields& other, std::true_type);
+
+    void
+    swap(basic_fields& other, std::false_type);
+
     set_t set_;
     list_t list_;
+    string_view method_;
+    string_view target_or_reason_;
     alloc_type alloc_;
 };
 

include/beast/http/impl/fields.ipp

@@ -18,147 +18,6 @@ namespace http {
 
 //------------------------------------------------------------------------------
 
-template<class Allocator>
-basic_fields<Allocator>::
-~basic_fields()
-{
-    delete_all();
-}
-
-//------------------------------------------------------------------------------
-
-template<class Allocator>
-inline
-string_view
-basic_fields<Allocator>::
-method_impl() const
-{
-    return (*this)[":method"];
-}
-
-template<class Allocator>
-inline
-string_view
-basic_fields<Allocator>::
-target_impl() const
-{
-    return (*this)[":target"];
-}
-
-template<class Allocator>
-inline
-string_view
-basic_fields<Allocator>::
-reason_impl() const
-{
-    return (*this)[":reason"];
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-method_impl(string_view s)
-{
-    if(s.empty())
-        this->erase(":method");
-    else
-        this->replace(":method", s);
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-target_impl(string_view s)
-{
-    if(s.empty())
-        this->erase(":target");
-    else
-        this->replace(":target", s);
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-reason_impl(string_view s)
-{
-    if(s.empty())
-        this->erase(":reason");
-    else
-        this->replace(":reason", s);
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-content_length_impl(std::uint64_t n)
-{
-    this->erase("Content-Length");
-    this->insert("Content-Length",
-        to_static_string(n));
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-connection_impl(close_t)
-{
-    auto it = find("Connection");
-    if(it == end())
-        this->insert("Connection", "close");
-    else
-        this->replace("Connection",
-            it->value().to_string() + ", close");
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-connection_impl(keep_alive_t)
-{
-    auto it = find("Connection");
-    if(it == end())
-        this->insert("Connection", "keep-alive");
-    else
-        this->replace("Connection",
-            it->value().to_string() + ", keep-alive");
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-connection_impl(upgrade_t)
-{
-    auto it = find("Connection");
-    if(it == end())
-        this->insert("Connection", "upgrade");
-    else
-        this->replace("Connection",
-            it->value().to_string() + ", upgrade");
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-chunked_impl()
-{
-    auto it = find("Transfer-Encoding");
-    if(it == end())
-        this->insert("Transfer-Encoding", "chunked");
-    else
-        this->replace("Transfer-Encoding",
-            it->value().to_string() + ", chunked");
-}
-
-//------------------------------------------------------------------------------
-
 template<class Allocator>
 class basic_fields<Allocator>::
     const_iterator
@@ -245,103 +104,63 @@ public:
     }
 };
 
+//------------------------------------------------------------------------------
+
 template<class Allocator>
-void
 basic_fields<Allocator>::
-delete_all()
+element::
+element(string_view name, string_view value)
+    : off_(static_cast<off_t>(name.size() + 2))
+    , len_(static_cast<off_t>(value.size()))
 {
-    for(auto it = list_.begin(); it != list_.end();)
+    char* p = reinterpret_cast<char*>(this + 1);
-        delete_element(*it++);
+    data.first = p;
+    data.off = off_;
+    data.len = len_;
+
+    p[off_-2] = ':';
+    p[off_-1] = ' ';
+    p[off_ + len_] = '\r';
+    p[off_ + len_ + 1] = '\n';
+    std::memcpy(p, name.data(), name.size());
+    std::memcpy(p + off_, value.data(), value.size());
 }
 
 template<class Allocator>
 inline
-void
+string_view
 basic_fields<Allocator>::
-move_assign(basic_fields& other, std::false_type)
+element::
+name() const
 {
-    if(alloc_ != other.alloc_)
+    return {reinterpret_cast<
-    {
+        char const*>(this + 1),
-        copy_from(other);
+            static_cast<std::size_t>(off_ - 2)};
-        other.clear();
-    }
-    else
-    {
-        set_ = std::move(other.set_);
-        list_ = std::move(other.list_);
-    }
 }
 
 template<class Allocator>
 inline
-void
+string_view
 basic_fields<Allocator>::
-move_assign(basic_fields& other, std::true_type)
+element::
+value() const
 {
-    alloc_ = std::move(other.alloc_);
+    return {reinterpret_cast<
-    set_ = std::move(other.set_);
+        char const*>(this + 1) + off_,
-    list_ = std::move(other.list_);
+            static_cast<std::size_t>(len_)};
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-copy_assign(basic_fields const& other, std::false_type)
-{
-    copy_from(other);
-}
-
-template<class Allocator>
-inline
-void
-basic_fields<Allocator>::
-copy_assign(basic_fields const& other, std::true_type)
-{
-    alloc_ = other.alloc_;
-    copy_from(other);
-}
-
-template<class Allocator>
-auto
-basic_fields<Allocator>::
-new_element(string_view name, string_view value) ->
-    element&
-{
-    if(name.size() + 2 >
-            (std::numeric_limits<off_t>::max)())
-        BOOST_THROW_EXCEPTION(std::length_error{
-            "field name too large"});
-    if(value.size() + 2 >
-            (std::numeric_limits<off_t>::max)())
-        BOOST_THROW_EXCEPTION(std::length_error{
-            "field value too large"});
-    value = detail::trim(value);
-    std::uint16_t const off =
-        static_cast<off_t>(name.size() + 2);
-    std::uint16_t const len =
-        static_cast<off_t>(value.size());
-    auto const p = alloc_traits::allocate(alloc_,
-        1 + (off + len + 2 + sizeof(element) - 1) /
-            sizeof(element));
-    alloc_traits::construct(alloc_, p, name, value);
-    return *p;
-}
-
-template<class Allocator>
-void
-basic_fields<Allocator>::
-delete_element(element& e)
-{
-    auto const n = 1 +
-        (e.data.off + e.data.len + 2 +
-            sizeof(element) - 1) / sizeof(element);
-    alloc_traits::destroy(alloc_, &e);
-    alloc_traits::deallocate(alloc_, &e, n);
 }
 
 //------------------------------------------------------------------------------
 
+template<class Allocator>
+basic_fields<Allocator>::
+~basic_fields()
+{
+    delete_list();
+    realloc_string(method_, {});
+    realloc_string(target_or_reason_, {});
+}
+
 template<class Allocator>
 basic_fields<Allocator>::
 basic_fields(Allocator const& alloc)
@@ -354,8 +173,48 @@ basic_fields<Allocator>::
 basic_fields(basic_fields&& other)
     : set_(std::move(other.set_))
     , list_(std::move(other.list_))
+    , method_(other.method_)
+    , target_or_reason_(other.target_or_reason_)
     , alloc_(std::move(other.alloc_))
 {
+    other.method_.clear();
+    other.target_or_reason_.clear();
+}
+
+template<class Allocator>
+basic_fields<Allocator>::
+basic_fields(basic_fields const& other)
+    : basic_fields(alloc_traits::
+        select_on_container_copy_construction(other.alloc_))
+{
+    copy_all(other);
+}
+
+template<class Allocator>
+basic_fields<Allocator>::
+basic_fields(basic_fields const& other,
+        Allocator const& alloc)
+    : alloc_(alloc)
+{
+    copy_all(other);
+}
+
+template<class Allocator>
+template<class OtherAlloc, class>
+basic_fields<Allocator>::
+basic_fields(basic_fields<OtherAlloc> const& other)
+{
+    copy_all(other);
+}
+
+template<class Allocator>
+template<class OtherAlloc, class>
+basic_fields<Allocator>::
+basic_fields(basic_fields<OtherAlloc> const& other,
+        Allocator const& alloc)
+    : alloc_(alloc)
+{
+    copy_all(other);
 }
 
 template<class Allocator>
@@ -366,53 +225,36 @@ operator=(basic_fields&& other) ->
 {
     if(this == &other)
         return *this;
-    clear();
+    move_assign(other, typename alloc_traits::
-    move_assign(other, std::integral_constant<bool,
+        propagate_on_container_move_assignment{});
-        alloc_traits::propagate_on_container_move_assignment::value>{});
     return *this;
 }
 
-template<class Allocator>
-basic_fields<Allocator>::
-basic_fields(basic_fields const& other)
-    : basic_fields(alloc_traits::
-        select_on_container_copy_construction(other.alloc_))
-{
-    copy_from(other);
-}
-
 template<class Allocator>
 auto
 basic_fields<Allocator>::
 operator=(basic_fields const& other) ->
     basic_fields&
 {
-    clear();
+    copy_assign(other, typename alloc_traits::
-    copy_assign(other, std::integral_constant<bool,
+        propagate_on_container_copy_assignment{});
-        alloc_traits::propagate_on_container_copy_assignment::value>{});
     return *this;
 }
 
 template<class Allocator>
-template<class OtherAlloc>
+template<class OtherAlloc, class>
-basic_fields<Allocator>::
-basic_fields(basic_fields<OtherAlloc> const& other)
-{
-    copy_from(other);
-}
-
-template<class Allocator>
-template<class OtherAlloc>
 auto
 basic_fields<Allocator>::
 operator=(basic_fields<OtherAlloc> const& other) ->
     basic_fields&
 {
-    clear();
+    clear_all();
-    copy_from(other);
+    copy_all(other);
     return *this;
 }
 
+//------------------------------------------------------------------------------
+
 template<class Allocator>
 std::size_t
 basic_fields<Allocator>::
@@ -453,9 +295,9 @@ void
 basic_fields<Allocator>::
 clear() noexcept
 {
-    delete_all();
+    delete_list();
-    list_.clear();
     set_.clear();
+    list_.clear();
 }
 
 template<class Allocator>
@@ -501,6 +343,328 @@ replace(string_view name, string_view value)
     insert(name, value);
 }
 
+//------------------------------------------------------------------------------
+
+template<class Allocator>
+inline
+string_view
+basic_fields<Allocator>::
+method_impl() const
+{
+    return method_;
+}
+
+template<class Allocator>
+inline
+string_view
+basic_fields<Allocator>::
+target_impl() const
+{
+    return target_or_reason_;
+}
+
+template<class Allocator>
+inline
+string_view
+basic_fields<Allocator>::
+reason_impl() const
+{
+    return target_or_reason_;
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+method_impl(string_view s)
+{
+    realloc_string(method_, s);
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+target_impl(string_view s)
+{
+    realloc_string(target_or_reason_, s);
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+reason_impl(string_view s)
+{
+    realloc_string(target_or_reason_, s);
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+content_length_impl(std::uint64_t n)
+{
+    this->erase("Content-Length");
+    this->insert("Content-Length",
+        to_static_string(n));
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+connection_impl(close_t)
+{
+    auto it = find("Connection");
+    if(it == end())
+        this->insert("Connection", "close");
+    else
+        this->replace("Connection",
+            it->value().to_string() + ", close");
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+connection_impl(keep_alive_t)
+{
+    auto it = find("Connection");
+    if(it == end())
+        this->insert("Connection", "keep-alive");
+    else
+        this->replace("Connection",
+            it->value().to_string() + ", keep-alive");
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+connection_impl(upgrade_t)
+{
+    auto it = find("Connection");
+    if(it == end())
+        this->insert("Connection", "upgrade");
+    else
+        this->replace("Connection",
+            it->value().to_string() + ", upgrade");
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+chunked_impl()
+{
+    auto it = find("Transfer-Encoding");
+    if(it == end())
+        this->insert("Transfer-Encoding", "chunked");
+    else
+        this->replace("Transfer-Encoding",
+            it->value().to_string() + ", chunked");
+}
+
+//------------------------------------------------------------------------------
+
+template<class Allocator>
+auto
+basic_fields<Allocator>::
+new_element(string_view name, string_view value) ->
+    element&
+{
+    if(name.size() + 2 >
+            (std::numeric_limits<off_t>::max)())
+        BOOST_THROW_EXCEPTION(std::length_error{
+            "field name too large"});
+    if(value.size() + 2 >
+            (std::numeric_limits<off_t>::max)())
+        BOOST_THROW_EXCEPTION(std::length_error{
+            "field value too large"});
+    value = detail::trim(value);
+    std::uint16_t const off =
+        static_cast<off_t>(name.size() + 2);
+    std::uint16_t const len =
+        static_cast<off_t>(value.size());
+    auto const p = alloc_traits::allocate(alloc_,
+        1 + (off + len + 2 + sizeof(element) - 1) /
+            sizeof(element));
+    alloc_traits::construct(alloc_, p, name, value);
+    return *p;
+}
+
+template<class Allocator>
+void
+basic_fields<Allocator>::
+delete_element(element& e)
+{
+    auto const n = 1 +
+        (e.data.off + e.data.len + 2 +
+            sizeof(element) - 1) / sizeof(element);
+    alloc_traits::destroy(alloc_, &e);
+    alloc_traits::deallocate(alloc_, &e, n);
+}
+
+template<class Allocator>
+void
+basic_fields<Allocator>::
+realloc_string(string_view& dest, string_view s)
+{
+    s = detail::trim(s);
+    if(dest.empty() && s.empty())
+        return;
+    auto a = typename std::allocator_traits<
+        Allocator>::template rebind_alloc<
+            char>(alloc_);
+    if(! dest.empty())
+    {
+        a.deallocate(const_cast<char*>(
+            dest.data()), dest.size());
+        dest.clear();
+    }
+    if(! s.empty())
+    {
+        auto const p = a.allocate(s.size());
+        std::memcpy(p, s.data(), s.size());
+        dest = {p, s.size()};
+    }
+}
+
+template<class Allocator>
+template<class OtherAlloc>
+void
+basic_fields<Allocator>::
+copy_all(basic_fields<OtherAlloc> const& other)
+{
+    for(auto const& e : other.list_)
+        insert(e.name(), e.value());
+    realloc_string(method_, other.method_);
+    realloc_string(target_or_reason_,
+        other.target_or_reason_);
+}
+
+template<class Allocator>
+void
+basic_fields<Allocator>::
+clear_all()
+{
+    clear();
+    realloc_string(method_, {});
+    realloc_string(target_or_reason_, {});
+}
+
+template<class Allocator>
+void
+basic_fields<Allocator>::
+delete_list()
+{
+    for(auto it = list_.begin(); it != list_.end();)
+        delete_element(*it++);
+}
+
+//------------------------------------------------------------------------------
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+move_assign(basic_fields& other, std::true_type)
+{
+    clear_all();
+    set_ = std::move(other.set_);
+    list_ = std::move(other.list_);
+    method_ = other.method_;
+    target_or_reason_ = other.target_or_reason_;
+    other.method_.clear();
+    other.target_or_reason_.clear();
+    alloc_ = other.alloc_;
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+move_assign(basic_fields& other, std::false_type)
+{
+    clear_all();
+    if(alloc_ != other.alloc_)
+    {
+        copy_all(other);
+        other.clear_all();
+    }
+    else
+    {
+        set_ = std::move(other.set_);
+        list_ = std::move(other.list_);
+        method_ = other.method_;
+        target_or_reason_ = other.target_or_reason_;
+        other.method_.clear();
+        other.target_or_reason_.clear();
+    }
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+copy_assign(basic_fields const& other, std::true_type)
+{
+    clear_all();
+    alloc_ = other.alloc_;
+    copy_all(other);
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+copy_assign(basic_fields const& other, std::false_type)
+{
+    clear_all();
+    copy_all(other);
+}
+
+template<class Allocator>
+void
+swap(basic_fields<Allocator>& lhs,
+    basic_fields<Allocator>& rhs)
+{
+    using alloc_traits = typename
+        basic_fields<Allocator>::alloc_traits;
+    lhs.swap(rhs, typename alloc_traits::
+        propagate_on_container_swap{});
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+swap(basic_fields& other, std::true_type)
+{
+    using std::swap;
+    swap(alloc_, other.alloc_);
+    swap(set_, other.set_);
+    swap(list_, other.list_);
+    swap(method_, other.method_);
+    swap(target_or_reason_, other.target_or_reason_);
+}
+
+template<class Allocator>
+inline
+void
+basic_fields<Allocator>::
+swap(basic_fields& other, std::false_type)
+{
+    BOOST_ASSERT(alloc_ == other.alloc_);
+    using std::swap;
+    swap(set_, other.set_);
+    swap(list_, other.list_);
+    swap(method_, other.method_);
+    swap(target_or_reason_, other.target_or_reason_);
+}
+
+
 } // http
 } // beast