| Message ID | b85d0bf5-7c8a-050a-1d5a-1f716edc2e1d@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | sized delete in _Temporary_buffer<> | expand |
On 18/07/19 07:41 +0200, François Dumont wrote: >As we adopted the sized deallocation in the new_allocator why not >doing the same in _Temporary_buffer<>. > > * include/bits/stl_tempbuf.h >(__detail::__return_temporary_buffer): New. > (~_Temporary_buffer()): Use latter. > (_Temporary_buffer(_FIterator, size_type)): Likewise. > >Tested w/o activating sized deallocation. I'll try to run tests with >this option activated. As the manual says, it's enabled by default for C++14 and later. >Ok to commit ? OK for trunk, thanks.
If I'm not mistaken this patch allocates N*sizeof(_Tp) bytes of storage and deallocates N bytes when sized deallocation is enabled? Shouldn't __return_temporary_buffer deallocate N*sizeof(_Tp) instead to match the value passed to new?
(2nd sent attempt as text this time.) Good spot, fixed with attached patch, committed as trivial. 2019-07-19 François Dumont <fdumont@gcc.gnu.org> * include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer): Fix sized deallocation size computation. On 7/19/19 9:46 PM, Morwenn Ed wrote: > If I'm not mistaken this patch allocates N*sizeof(_Tp) bytes of > storage and deallocates N bytes when sized deallocation is enabled? > > Shouldn't __return_temporary_buffer deallocate N*sizeof(_Tp) instead > to match the value passed to new? > > ------------------------------------------------------------------------ > *De :* libstdc++-owner@gcc.gnu.org <libstdc++-owner@gcc.gnu.org> de la > part de François Dumont <frs.dumont@gmail.com> > *Envoyé :* jeudi 18 juillet 2019 07:41 > *À :* libstdc++@gcc.gnu.org <libstdc++@gcc.gnu.org>; gcc-patches > <gcc-patches@gcc.gnu.org> > *Objet :* sized delete in _Temporary_buffer<> > As we adopted the sized deallocation in the new_allocator why not doing > the same in _Temporary_buffer<>. > > * include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer): > New. > (~_Temporary_buffer()): Use latter. > (_Temporary_buffer(_FIterator, size_type)): Likewise. > > Tested w/o activating sized deallocation. I'll try to run tests with > this option activated. > > Ok to commit ? > > François >
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h index b6ad9ee6a46..bb7c2cd1334 100644 --- a/libstdc++-v3/include/bits/stl_tempbuf.h +++ b/libstdc++-v3/include/bits/stl_tempbuf.h @@ -63,6 +63,21 @@ namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION + namespace __detail + { + template<typename _Tp> + inline void + __return_temporary_buffer(_Tp* __p, + size_t __len __attribute__((__unused__))) + { +#if __cpp_sized_deallocation + ::operator delete(__p, __len); +#else + ::operator delete(__p); +#endif + } + } + /** * @brief Allocates a temporary buffer. * @param __len The number of objects of type Tp. @@ -112,7 +127,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return_temporary_buffer(_Tp* __p) { ::operator delete(__p); } - /** * This class is used in two places: stl_algo.h and ext/memory, * where it is wrapped as the temporary_buffer class. See @@ -165,7 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION ~_Temporary_buffer() { std::_Destroy(_M_buffer, _M_buffer + _M_len); - std::return_temporary_buffer(_M_buffer); + std::__detail::__return_temporary_buffer(_M_buffer, _M_len); } private: @@ -185,7 +199,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __ucr(_Pointer __first, _Pointer __last, _ForwardIterator __seed) { - if(__first == __last) + if (__first == __last) return; _Pointer __cur = __first; @@ -244,22 +258,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Temporary_buffer(_ForwardIterator __seed, size_type __original_len) : _M_original_len(__original_len), _M_len(0), _M_buffer(0) { - __try - { - std::pair<pointer, size_type> __p(std::get_temporary_buffer< - value_type>(_M_original_len)); - _M_buffer = __p.first; - _M_len = __p.second; - if (_M_buffer) - std::__uninitialized_construct_buf(_M_buffer, _M_buffer + _M_len, - __seed); - } - __catch(...) + std::pair<pointer, size_type> __p( + std::get_temporary_buffer<value_type>(_M_original_len)); + + if (__p.first) { - std::return_temporary_buffer(_M_buffer); - _M_buffer = 0; - _M_len = 0; - __throw_exception_again; + __try + { + std::__uninitialized_construct_buf(__p.first, __p.first + __p.second, + __seed); + _M_buffer = __p.first; + _M_len = __p.second; + } + __catch(...) + { + std::__detail::__return_temporary_buffer(__p.first, __p.second); + __throw_exception_again; + } } }