diff --git a/include/qpdf/Buffer.hh b/include/qpdf/Buffer.hh index 8e2fc8d..928821e 100644 --- a/include/qpdf/Buffer.hh +++ b/include/qpdf/Buffer.hh @@ -46,15 +46,13 @@ class Buffer QPDF_DLL Buffer(std::string& content); - [[deprecated("Move Buffer or use Buffer::copy instead")]] QPDF_DLL Buffer(Buffer const&); - [[deprecated("Move Buffer or use Buffer::copy instead")]] QPDF_DLL Buffer& - operator=(Buffer const&); - QPDF_DLL Buffer(Buffer&&) noexcept; QPDF_DLL Buffer& operator=(Buffer&&) noexcept; QPDF_DLL + ~Buffer(); + QPDF_DLL size_t getSize() const; QPDF_DLL unsigned char const* getBuffer() const; @@ -65,29 +63,8 @@ class Buffer QPDF_DLL Buffer copy() const; - // Only used during CI testing. - // ABI: remove when removing copy constructor / assignment operator - static void setTestMode() noexcept; - private: - class Members - { - friend class Buffer; - - public: - QPDF_DLL - ~Members(); - - private: - Members(size_t size, unsigned char* buf, bool own_memory); - Members(std::string&& content); - Members(Members const&) = delete; - - std::string str; - bool own_memory; - size_t size; - unsigned char* buf; - }; + class Members; void copy(Buffer const&); diff --git a/libqpdf/Buffer.cc b/libqpdf/Buffer.cc index 20453a4..26041af 100644 --- a/libqpdf/Buffer.cc +++ b/libqpdf/Buffer.cc @@ -4,16 +4,23 @@ #include -bool test_mode = false; - -// During CI the Buffer copy constructor and copy assignment operator throw an assertion error to -// detect their accidental use. Call setTestMode to surpress the assertion errors for testing of -// copy construction and assignment. -void -Buffer::setTestMode() noexcept +class Buffer::Members { - test_mode = true; -} + friend class Buffer; + + public: + ~Members(); + + private: + Members(size_t size, unsigned char* buf, bool own_memory); + Members(std::string&& content); + Members(Members const&) = delete; + + std::string str; + bool own_memory; + size_t size; + unsigned char* buf; +}; Buffer::Members::Members(size_t size, unsigned char* buf, bool own_memory) : own_memory(own_memory), @@ -67,20 +74,6 @@ Buffer::Buffer(std::string& content) : { } -Buffer::Buffer(Buffer const& rhs) -{ - assert(test_mode); - copy(rhs); -} - -Buffer& -Buffer::operator=(Buffer const& rhs) -{ - assert(test_mode); - copy(rhs); - return *this; -} - Buffer::Buffer(Buffer&& rhs) noexcept : m(std::move(rhs.m)) { @@ -93,6 +86,8 @@ Buffer::operator=(Buffer&& rhs) noexcept return *this; } +Buffer::~Buffer() = default; + void Buffer::copy(Buffer const& rhs) { diff --git a/libtests/buffer.cc b/libtests/buffer.cc index 1b87bb7..4326901 100644 --- a/libtests/buffer.cc +++ b/libtests/buffer.cc @@ -18,130 +18,65 @@ uc(char const* s) int main() { - { - // Test that buffers can be copied by value using Buffer::copy. - Buffer bc1(2); - unsigned char* bc1p = bc1.getBuffer(); - bc1p[0] = 'Q'; - bc1p[1] = 'W'; - Buffer bc2(bc1.copy()); - bc1p[0] = 'R'; - unsigned char* bc2p = bc2.getBuffer(); - assert(bc2p != bc1p); - assert(bc2p[0] == 'Q'); - assert(bc2p[1] == 'W'); - bc2 = bc1.copy(); - bc2p = bc2.getBuffer(); - assert(bc2p != bc1p); - assert(bc2p[0] == 'R'); - assert(bc2p[1] == 'W'); - - // Test Buffer(std:string&&) - Buffer bc3("QW"); - unsigned char* bc3p = bc3.getBuffer(); - Buffer bc4(bc3.copy()); - bc3p[0] = 'R'; - unsigned char* bc4p = bc4.getBuffer(); - assert(bc4p != bc3p); - assert(bc4p[0] == 'Q'); - assert(bc4p[1] == 'W'); - bc4 = bc3.copy(); - bc4p = bc4.getBuffer(); - assert(bc4p != bc3p); - assert(bc4p[0] == 'R'); - assert(bc4p[1] == 'W'); - } - -#ifdef _MSC_VER -# pragma warning(disable : 4996) -#endif -#if (defined(__GNUC__) || defined(__clang__)) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" -#endif - { - // Test that buffers can be copied by value using copy construction / assignment. - Buffer::setTestMode(); - Buffer bc1(2); - unsigned char* bc1p = bc1.getBuffer(); - bc1p[0] = 'Q'; - bc1p[1] = 'W'; - Buffer bc2(bc1); - bc1p[0] = 'R'; - unsigned char* bc2p = bc2.getBuffer(); - assert(bc2p != bc1p); - assert(bc2p[0] == 'Q'); - assert(bc2p[1] == 'W'); - bc2 = bc1; - bc2p = bc2.getBuffer(); - assert(bc2p != bc1p); - assert(bc2p[0] == 'R'); - assert(bc2p[1] == 'W'); - - // Test Buffer(std:string&&) - Buffer bc3("QW"); - unsigned char* bc3p = bc3.getBuffer(); - Buffer bc4(bc3); - bc3p[0] = 'R'; - unsigned char* bc4p = bc4.getBuffer(); - assert(bc4p != bc3p); - assert(bc4p[0] == 'Q'); - assert(bc4p[1] == 'W'); - bc4 = bc3; - bc4p = bc4.getBuffer(); - assert(bc4p != bc3p); - assert(bc2p[0] == 'R'); - assert(bc2p[1] == 'W'); - - // Test Buffer(std:string&) - std::string s{"QW"}; - Buffer bc5(s); - unsigned char* bc5p = bc5.getBuffer(); - Buffer bc6(bc5); - bc5p[0] = 'R'; - unsigned char* bc6p = bc6.getBuffer(); - assert(bc6p != bc5p); - assert(bc6p[0] == 'Q'); - assert(bc6p[1] == 'W'); - bc6 = bc5; - bc6p = bc6.getBuffer(); - assert(bc6p != bc5p); - assert(bc2p[0] == 'R'); - assert(bc2p[1] == 'W'); - } -#if (defined(__GNUC__) || defined(__clang__)) -# pragma GCC diagnostic pop -#endif - - { - // Test that buffers can be moved. - Buffer bm1(2); - unsigned char* bm1p = bm1.getBuffer(); - bm1p[0] = 'Q'; - bm1p[1] = 'W'; - Buffer bm2(std::move(bm1)); - bm1p[0] = 'R'; - unsigned char* bm2p = bm2.getBuffer(); - assert(bm2p == bm1p); - assert(bm2p[0] == 'R'); - - Buffer bm3 = std::move(bm2); - unsigned char* bm3p = bm3.getBuffer(); - assert(bm3p == bm2p); - - // Test Buffer(dtd::string&&) - Buffer bm4("QW"); - unsigned char* bm4p = bm4.getBuffer(); - Buffer bm5(std::move(bm4)); - bm4p[0] = 'R'; - unsigned char* bm5p = bm5.getBuffer(); - assert(bm5p == bm4p); - assert(bm5p[0] == 'R'); - - Buffer bm6 = std::move(bm5); - unsigned char* bm6p = bm6.getBuffer(); - assert(bm6p == bm5p); - } + // Test that buffers can be copied by value using Buffer::copy. + Buffer bc1(2); + unsigned char* bc1p = bc1.getBuffer(); + bc1p[0] = 'Q'; + bc1p[1] = 'W'; + Buffer bc2(bc1.copy()); + bc1p[0] = 'R'; + unsigned char* bc2p = bc2.getBuffer(); + assert(bc2p != bc1p); + assert(bc2p[0] == 'Q'); + assert(bc2p[1] == 'W'); + bc2 = bc1.copy(); + bc2p = bc2.getBuffer(); + assert(bc2p != bc1p); + assert(bc2p[0] == 'R'); + assert(bc2p[1] == 'W'); + + // Test Buffer(std:string&&) + Buffer bc3("QW"); + unsigned char* bc3p = bc3.getBuffer(); + Buffer bc4(bc3.copy()); + bc3p[0] = 'R'; + unsigned char* bc4p = bc4.getBuffer(); + assert(bc4p != bc3p); + assert(bc4p[0] == 'Q'); + assert(bc4p[1] == 'W'); + bc4 = bc3.copy(); + bc4p = bc4.getBuffer(); + assert(bc4p != bc3p); + assert(bc4p[0] == 'R'); + assert(bc4p[1] == 'W'); + + // Test that buffers can be moved. + Buffer bm1(2); + unsigned char* bm1p = bm1.getBuffer(); + bm1p[0] = 'Q'; + bm1p[1] = 'W'; + Buffer bm2(std::move(bm1)); + bm1p[0] = 'R'; + unsigned char* bm2p = bm2.getBuffer(); + assert(bm2p == bm1p); + assert(bm2p[0] == 'R'); + + Buffer bm3 = std::move(bm2); + unsigned char* bm3p = bm3.getBuffer(); + assert(bm3p == bm2p); + + // Test Buffer(dtd::string&&) + Buffer bm4("QW"); + unsigned char* bm4p = bm4.getBuffer(); + Buffer bm5(std::move(bm4)); + bm4p[0] = 'R'; + unsigned char* bm5p = bm5.getBuffer(); + assert(bm5p == bm4p); + assert(bm5p[0] == 'R'); + + Buffer bm6 = std::move(bm5); + unsigned char* bm6p = bm6.getBuffer(); + assert(bm6p == bm5p); try { Pl_Discard discard; diff --git a/manual/release-notes.rst b/manual/release-notes.rst index b60ed19..c5522ba 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -9,18 +9,20 @@ For a detailed list of changes, please see the file Planned changes for future 12.x (subject to change): - - ``Buffer`` copy constructor and assignment operator will be - removed. ``Buffer`` copy operations are expensive as they always - involve copying the buffer content. Use ``buffer2 = - buffer1.copy();`` or ``Buffer buffer2{buffer1.copy()};`` to make - it explicit that copying is intended. - - ``QIntC.hh`` contains the type ``substract``, which will be fixed to ``subtract``. (Not enabled with ``FUTURE`` option.) .. x.y.z: not yet released 12.0.0: not yet released + - API: breaking changes + + - ``Buffer`` copy constructor and assignment operator have been + removed. ``Buffer`` copy operations are expensive as they always + involve copying the buffer content. Use ``buffer2 = buffer1.copy();`` + or ``Buffer buffer2{buffer1.copy()};`` to make it explicit that + copying is intended. + - Library Enhancements - ``QPDFObjectHandle`` supports move construction/assignment.