That would seem like a duplication of responsibilities between data_ and the new proposed string member. IMO the destructor should have a way to know if it owns the memory, and should de allocate it on destruction of object.
On Sun, 30 Sep 2018 at 3:45 PM, Wes McKinney <[email protected]> wrote: > *@wesm* commented on this pull request. > ------------------------------ > > In cpp/src/arrow/buffer.h > <https://github.com/apache/arrow/pull/2660#discussion_r221454578>: > > > @@ -259,6 +259,20 @@ class ARROW_EXPORT ResizableBuffer : public > > MutableBuffer { > ResizableBuffer(uint8_t* data, int64_t size) : MutableBuffer(data, size) {} > }; > > +/// \class STLStringBuffer > +/// \brief A Buffer which owns the memory pointed to by a std::string > +class ARROW_EXPORT STLStringBuffer : public Buffer { > +public: > + STLStringBuffer(std::string& data) > + : Buffer(reinterpret_cast<const uint8_t*>(std::move(data.c_str())), > + static_cast<int64_t>(data.size())) { > > Buffer won't deallocate this memory, though, because it has a trivial > destructor. The safest thing would be to use a std::string to transfer > the internal state of the input std::string and have deallocation handled > by RAII > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/apache/arrow/pull/2660#discussion_r221454578>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABpO49K5mMkCGlnsI4Me05aXdc6Nicqzks5ugJm4gaJpZM4XAzaS> > . > -- Regards, Atri *l'apprenant* [ Full content available at: https://github.com/apache/arrow/pull/2660 ] This message was relayed via gitbox.apache.org for [email protected]
