On 24/11/18 22:58 +0100, François Dumont wrote:
Tests have shown a regression. I was building the _ReuseOrAllocNode instance too early, node ownership was transfered but was still owned by _Hashtable instance.

This new version pass all tests.

This is why it's worth waiting until tests have run before sending a
patch for review.

Ok to commit ?

OK, but please rename _M_replicate to _M_do_assign or _M_assign_impl
or something that makes it clear this is part of the assignment
operation. The name "replicate" isn't clear.

A comment on the declaration of the new function could be helpful too.

Thanks for finding this leak. I think it's worth backporting the fix,
but for gcc-7-branch and gcc-8-branch it should be just:

--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -1223,6 +1223,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                       [&__roan](__node_type* __n)
                       { return __roan(std::move_if_noexcept(__n->_M_v())); });
             __ht.clear();
+             if (__former_buckets)
+               _M_deallocate_buckets(__former_buckets, __former_bucket_count);
           }
         __catch(...)
           {

(Plus the improvements to the tests, of course).



Reply via email to