Merge pull request #983 from m-holger/buffer

Add new Buffer method copy and deprecate copy constructor / assignment operator

Merge pull request #983 from m-holger/buffer
Add new Buffer method copy and deprecate copy constructor / assignment operator
Jay Berkenbilt · GitHub
2 parents 463953bc 0f2ef5e8
Showing 4 changed files with 69 additions and 6 deletions
include/qpdf/Buffer.hh
libqpdf/Buffer.cc
libqpdf/Pl_LZWDecoder.cc
libtests/buffer.cc
@@ -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
     {
+#include <qpdf/assert_test.h>
+
 #include <qpdf/Buffer.hh>
 #include <cstring>
+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;
+}
@@ -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
@@ -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.