diff --git a/include/qpdf/Buffer.hh b/include/qpdf/Buffer.hh index 979d57d..d667c1a 100644 --- a/include/qpdf/Buffer.hh +++ b/include/qpdf/Buffer.hh @@ -25,6 +25,7 @@ #include #include #include +#include class Buffer { @@ -66,11 +67,34 @@ class Buffer QPDF_DLL Buffer copy() const; + // Move the content of the Buffer. After calling this method, the Buffer will be empty if the + // buffer owns its memory. Otherwise, the Buffer will be unchanged. + QPDF_DLL + std::string move(); + + // Return a string_view to the data. + QPDF_DLL + std::string_view view() const; + + // Return a pointer to the data. NB: Unlike getBuffer, this method returns a valid pointer even + // if the Buffer is empty. + QPDF_DLL + char const* data() const; + + // Return a pointer to the data. NB: Unlike getBuffer, this method returns a valid pointer even + // if the Buffer is empty. + QPDF_DLL + char* data(); + + QPDF_DLL + bool empty() const; + + QPDF_DLL + size_t size() const; + private: class Members; - void copy(Buffer const&); - std::unique_ptr m; }; diff --git a/libqpdf/Buffer.cc b/libqpdf/Buffer.cc index 26041af..5971b25 100644 --- a/libqpdf/Buffer.cc +++ b/libqpdf/Buffer.cc @@ -2,75 +2,55 @@ #include -#include - class Buffer::Members { friend class Buffer; public: - ~Members(); - - private: - Members(size_t size, unsigned char* buf, bool own_memory); - Members(std::string&& content); + Members() = default; + // Constructor for Buffers that don't own the memory. + Members(size_t size, char* buf) : + size(size), + buf(buf) + { + } + Members(std::string&& content) : + str(std::move(content)), + size(str.size()), + buf(str.data()) + { + } Members(Members const&) = delete; + ~Members() = default; + private: std::string str; - bool own_memory; size_t size; - unsigned char* buf; + char* buf; }; -Buffer::Members::Members(size_t size, unsigned char* buf, bool own_memory) : - own_memory(own_memory), - size(size), - buf(nullptr) -{ - if (own_memory) { - this->buf = (size ? new unsigned char[size] : nullptr); - } else { - this->buf = buf; - } -} - -Buffer::Members::Members(std::string&& content) : - str(std::move(content)), - own_memory(false), - size(str.size()), - buf(reinterpret_cast(str.data())) -{ -} - -Buffer::Members::~Members() -{ - if (this->own_memory) { - delete[] this->buf; - } -} - Buffer::Buffer() : - m(new Members(0, nullptr, true)) + m(std::make_unique()) { } Buffer::Buffer(size_t size) : - m(new Members(size, nullptr, true)) + m(std::make_unique(std::string(size, '\0'))) { } Buffer::Buffer(std::string&& content) : - m(new Members(std::move(content))) + m(std::make_unique(std::move(content))) { } Buffer::Buffer(unsigned char* buf, size_t size) : - m(new Members(size, buf, false)) + m(std::make_unique(size, reinterpret_cast(buf))) { } Buffer::Buffer(std::string& content) : - m(new Members(content.size(), reinterpret_cast(content.data()), false)) + m(std::make_unique(content.size(), content.data())) { } @@ -88,17 +68,6 @@ Buffer::operator=(Buffer&& rhs) noexcept Buffer::~Buffer() = default; -void -Buffer::copy(Buffer const& rhs) -{ - if (this != &rhs) { - m = std::unique_ptr(new Members(rhs.m->size, nullptr, true)); - if (m->size) { - memcpy(m->buf, rhs.m->buf, m->size); - } - } -} - size_t Buffer::getSize() const { @@ -108,21 +77,67 @@ Buffer::getSize() const unsigned char const* Buffer::getBuffer() const { - return m->buf; + return reinterpret_cast(m->buf); } unsigned char* Buffer::getBuffer() { - return m->buf; + return reinterpret_cast(m->buf); } Buffer Buffer::copy() const { - auto result = Buffer(m->size); - if (m->size) { - memcpy(result.m->buf, m->buf, m->size); + if (m->size == 0) { + return {}; + } + return {std::string(m->buf, m->size)}; +} + +std::string +Buffer::move() +{ + if (m->size == 0) { + return {}; } - return result; + if (!m->str.empty()) { + m->size = 0; + m->buf = nullptr; + return std::move(m->str); + } + return {m->buf, m->size}; +} + +std::string_view +Buffer::view() const +{ + if (!m->buf) { + return {}; + } + return {m->buf, m->size}; +} + +char const* +Buffer::data() const +{ + return m->buf ? m->buf : m->str.data(); +} + +char* +Buffer::data() +{ + return m->buf ? m->buf : m->str.data(); +} + +bool +Buffer::empty() const +{ + return m->size == 0; +} + +size_t +Buffer::size() const +{ + return m->size; } diff --git a/libtests/buffer.cc b/libtests/buffer.cc index 295597c..f092255 100644 --- a/libtests/buffer.cc +++ b/libtests/buffer.cc @@ -35,6 +35,37 @@ main() assert(bc2p[0] == 'R'); assert(bc2p[1] == 'W'); + // Test move, view, data, empty and size methods + assert(bc1.view() == "RW"); + assert(std::string_view(bc1.data(), bc1.getSize()) == "RW"); + *bc1.data() = 'Z'; + assert(!bc1.empty()); + assert(bc1.size() == 2); + auto s1 = bc1.move(); + assert(bc1.empty()); + assert(bc1.size() == 0); + assert(!bc1.getBuffer()); + assert(bc1.getSize() == 0); + assert(s1 == "ZW"); + assert(bc1.view().empty()); + assert(bc1.data()); + assert(Buffer(s1).move() == "ZW"); + assert(s1 == "ZW"); + + // Test const methods + const Buffer cb(s1); + assert(*cb.data() == 'Z'); + assert(*(cb.getBuffer() + 1) == 'W'); + + // Test constructors + assert(Buffer().empty()); + assert(Buffer().copy().empty()); + assert(!Buffer().getBuffer()); + assert(Buffer().data()); + assert(Buffer().move().empty()); + assert(Buffer(s1).size() == 2); + assert(*Buffer(s1).data() == 'Z'); + // Test Buffer(std:string&&) Buffer bc3("QW"); unsigned char* bc3p = bc3.getBuffer(); diff --git a/manual/release-notes.rst b/manual/release-notes.rst index baff92c..a68d42f 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -38,9 +38,15 @@ more detail. overhead of repeatedly validating the underlying document structure and/or building internal caches. If the underlying document structure is directly modified (without the use of DocumentHelpers), the - ``validate`` methods revalidates the structure and resynchronizes any + ``validate`` methods revalidate the structure and resynchronize any internal caches. + - Add new ``Buffer`` methods ``move``, ``view``, ``data``, ``size`` and + ``empty``. The new methods present the ``Buffer`` as a ``char`` (rather + than ``unsigned char``) container and facilitate the efficient moving + of its content into a `std::string``. + + - CLI Enhancements - Disallow option :qpdf:ref:`--deterministic-id` to be used together