Commit 0f2ef5e85bce0d64683e8071151711f21fa3e052
1 parent
acd0acf1
Add new Buffer method copy and deprecate copy constructor / assignment operator
Also fix accidental Buffer copy in Pl_LZWDecoder::addToTable.
Showing
4 changed files
with
69 additions
and
6 deletions
include/qpdf/Buffer.hh
| ... | ... | @@ -41,10 +41,10 @@ class Buffer |
| 41 | 41 | QPDF_DLL |
| 42 | 42 | Buffer(unsigned char* buf, size_t size); |
| 43 | 43 | |
| 44 | - QPDF_DLL | |
| 45 | - Buffer(Buffer const&); | |
| 46 | - QPDF_DLL | |
| 47 | - Buffer& operator=(Buffer const&); | |
| 44 | + [[deprecated("Move Buffer or use Buffer::copy instead")]] QPDF_DLL Buffer(Buffer const&); | |
| 45 | + [[deprecated("Move Buffer or use Buffer::copy instead")]] QPDF_DLL Buffer& | |
| 46 | + operator=(Buffer const&); | |
| 47 | + | |
| 48 | 48 | QPDF_DLL |
| 49 | 49 | Buffer(Buffer&&) noexcept; |
| 50 | 50 | QPDF_DLL |
| ... | ... | @@ -56,6 +56,14 @@ class Buffer |
| 56 | 56 | QPDF_DLL |
| 57 | 57 | unsigned char* getBuffer(); |
| 58 | 58 | |
| 59 | + // Create a new copy of the Buffer. The new Buffer owns an independent copy of the data. | |
| 60 | + QPDF_DLL | |
| 61 | + Buffer copy() const; | |
| 62 | + | |
| 63 | + // Only used during CI testing. | |
| 64 | + // ABI: remove when removing copy constructor / assignment operator | |
| 65 | + static void setTestMode() noexcept; | |
| 66 | + | |
| 59 | 67 | private: |
| 60 | 68 | class Members |
| 61 | 69 | { | ... | ... |
libqpdf/Buffer.cc
| 1 | +#include <qpdf/assert_test.h> | |
| 2 | + | |
| 1 | 3 | #include <qpdf/Buffer.hh> |
| 2 | 4 | |
| 3 | 5 | #include <cstring> |
| 4 | 6 | |
| 7 | +bool test_mode = false; | |
| 8 | + | |
| 9 | +// During CI the Buffer copy constructor and copy assignment operator throw an assertion error to | |
| 10 | +// detect their accidental use. Call setTestMode to surpress the assertion errors for testing of | |
| 11 | +// copy construction and assignment. | |
| 12 | +void | |
| 13 | +Buffer::setTestMode() noexcept | |
| 14 | +{ | |
| 15 | + test_mode = true; | |
| 16 | +} | |
| 17 | + | |
| 5 | 18 | Buffer::Members::Members(size_t size, unsigned char* buf, bool own_memory) : |
| 6 | 19 | own_memory(own_memory), |
| 7 | 20 | size(size), |
| ... | ... | @@ -38,12 +51,14 @@ Buffer::Buffer(unsigned char* buf, size_t size) : |
| 38 | 51 | |
| 39 | 52 | Buffer::Buffer(Buffer const& rhs) |
| 40 | 53 | { |
| 54 | + assert(test_mode); | |
| 41 | 55 | copy(rhs); |
| 42 | 56 | } |
| 43 | 57 | |
| 44 | 58 | Buffer& |
| 45 | 59 | Buffer::operator=(Buffer const& rhs) |
| 46 | 60 | { |
| 61 | + assert(test_mode); | |
| 47 | 62 | copy(rhs); |
| 48 | 63 | return *this; |
| 49 | 64 | } |
| ... | ... | @@ -88,3 +103,13 @@ Buffer::getBuffer() |
| 88 | 103 | { |
| 89 | 104 | return m->buf; |
| 90 | 105 | } |
| 106 | + | |
| 107 | +Buffer | |
| 108 | +Buffer::copy() const | |
| 109 | +{ | |
| 110 | + auto result = Buffer(m->size); | |
| 111 | + if (m->size) { | |
| 112 | + memcpy(result.m->buf, m->buf, m->size); | |
| 113 | + } | |
| 114 | + return result; | |
| 115 | +} | ... | ... |
libqpdf/Pl_LZWDecoder.cc
| ... | ... | @@ -129,7 +129,7 @@ Pl_LZWDecoder::addToTable(unsigned char next) |
| 129 | 129 | unsigned char* new_data = entry.getBuffer(); |
| 130 | 130 | memcpy(new_data, last_data, last_size); |
| 131 | 131 | new_data[last_size] = next; |
| 132 | - this->table.push_back(entry); | |
| 132 | + this->table.push_back(std::move(entry)); | |
| 133 | 133 | } |
| 134 | 134 | |
| 135 | 135 | void | ... | ... |
libtests/buffer.cc
| ... | ... | @@ -19,7 +19,34 @@ int |
| 19 | 19 | main() |
| 20 | 20 | { |
| 21 | 21 | { |
| 22 | - // Test that buffers can be copied by value. | |
| 22 | + // Test that buffers can be copied by value using Buffer::copy. | |
| 23 | + Buffer bc1(2); | |
| 24 | + unsigned char* bc1p = bc1.getBuffer(); | |
| 25 | + bc1p[0] = 'Q'; | |
| 26 | + bc1p[1] = 'W'; | |
| 27 | + Buffer bc2(bc1.copy()); | |
| 28 | + bc1p[0] = 'R'; | |
| 29 | + unsigned char* bc2p = bc2.getBuffer(); | |
| 30 | + assert(bc2p != bc1p); | |
| 31 | + assert(bc2p[0] == 'Q'); | |
| 32 | + assert(bc2p[1] == 'W'); | |
| 33 | + bc2 = bc1.copy(); | |
| 34 | + bc2p = bc2.getBuffer(); | |
| 35 | + assert(bc2p != bc1p); | |
| 36 | + assert(bc2p[0] == 'R'); | |
| 37 | + assert(bc2p[1] == 'W'); | |
| 38 | + } | |
| 39 | + | |
| 40 | +#ifdef _MSC_VER | |
| 41 | +# pragma warning(disable : 4996) | |
| 42 | +#endif | |
| 43 | +#if (defined(__GNUC__) || defined(__clang__)) | |
| 44 | +# pragma GCC diagnostic push | |
| 45 | +# pragma GCC diagnostic ignored "-Wdeprecated-declarations" | |
| 46 | +#endif | |
| 47 | + { | |
| 48 | + // Test that buffers can be copied by value using copy construction / assignment. | |
| 49 | + Buffer::setTestMode(); | |
| 23 | 50 | Buffer bc1(2); |
| 24 | 51 | unsigned char* bc1p = bc1.getBuffer(); |
| 25 | 52 | bc1p[0] = 'Q'; |
| ... | ... | @@ -36,6 +63,9 @@ main() |
| 36 | 63 | assert(bc2p[0] == 'R'); |
| 37 | 64 | assert(bc2p[1] == 'W'); |
| 38 | 65 | } |
| 66 | +#if (defined(__GNUC__) || defined(__clang__)) | |
| 67 | +# pragma GCC diagnostic pop | |
| 68 | +#endif | |
| 39 | 69 | |
| 40 | 70 | { |
| 41 | 71 | // Test that buffers can be moved. | ... | ... |