diff --git a/include/qpdf/Buffer.hh b/include/qpdf/Buffer.hh index 719a6bd5..c0669c6c 100644 --- a/include/qpdf/Buffer.hh +++ b/include/qpdf/Buffer.hh @@ -41,10 +41,10 @@ class Buffer QPDF_DLL Buffer(unsigned char* buf, size_t size); - QPDF_DLL - Buffer(Buffer const&); - QPDF_DLL - Buffer& operator=(Buffer const&); + [[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 @@ -56,6 +56,14 @@ class Buffer QPDF_DLL unsigned char* getBuffer(); + // Create a new copy of the Buffer. The new Buffer owns an independent copy of the data. + 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 { diff --git a/libqpdf/Buffer.cc b/libqpdf/Buffer.cc index ae04fbc8..3dddd5db 100644 --- a/libqpdf/Buffer.cc +++ b/libqpdf/Buffer.cc @@ -1,7 +1,20 @@ +#include + #include #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 +{ + test_mode = true; +} + Buffer::Members::Members(size_t size, unsigned char* buf, bool own_memory) : own_memory(own_memory), size(size), @@ -38,12 +51,14 @@ Buffer::Buffer(unsigned char* buf, size_t size) : Buffer::Buffer(Buffer const& rhs) { + assert(test_mode); copy(rhs); } Buffer& Buffer::operator=(Buffer const& rhs) { + assert(test_mode); copy(rhs); return *this; } @@ -88,3 +103,13 @@ Buffer::getBuffer() { return m->buf; } + +Buffer +Buffer::copy() const +{ + auto result = Buffer(m->size); + if (m->size) { + memcpy(result.m->buf, m->buf, m->size); + } + return result; +} diff --git a/libqpdf/Pl_LZWDecoder.cc b/libqpdf/Pl_LZWDecoder.cc index 4ffcaa3f..9abb69cd 100644 --- a/libqpdf/Pl_LZWDecoder.cc +++ b/libqpdf/Pl_LZWDecoder.cc @@ -129,7 +129,7 @@ Pl_LZWDecoder::addToTable(unsigned char next) unsigned char* new_data = entry.getBuffer(); memcpy(new_data, last_data, last_size); new_data[last_size] = next; - this->table.push_back(entry); + this->table.push_back(std::move(entry)); } void diff --git a/libtests/buffer.cc b/libtests/buffer.cc index 86a52899..e62a37ca 100644 --- a/libtests/buffer.cc +++ b/libtests/buffer.cc @@ -19,7 +19,34 @@ int main() { { - // Test that buffers can be copied by value. + // 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'); + } + +#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'; @@ -36,6 +63,9 @@ main() assert(bc2p[0] == 'R'); assert(bc2p[1] == 'W'); } +#if (defined(__GNUC__) || defined(__clang__)) +# pragma GCC diagnostic pop +#endif { // Test that buffers can be moved.