From 81bf3e68b9edb6fc8635ab5520f86df9c8d6ef04 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sun, 12 Dec 2021 17:04:26 +0100 Subject: [PATCH] Make QRingBuffer a move-only type There's no sense in copying a ring buffer. Moving is enough. This marks an important step on the way to preventing accidental copies of ring buffer content, because the 'QList buffers' member can now no longer be implicitly shared. While the compiler will still emit the code for detach()ing, it will now never be executed. Pick-to: 6.3 Change-Id: I968bfe3e50c46720ed4baca55c99c1f9c518f653 Reviewed-by: Thiago Macieira --- src/corelib/io/qiodevice.cpp | 14 +++++++++----- src/corelib/tools/qringbuffer_p.h | 4 ++++ .../corelib/tools/qringbuffer/tst_qringbuffer.cpp | 15 +++++++++++++-- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/corelib/io/qiodevice.cpp b/src/corelib/io/qiodevice.cpp index e362535852..57d26884ee 100644 --- a/src/corelib/io/qiodevice.cpp +++ b/src/corelib/io/qiodevice.cpp @@ -703,15 +703,18 @@ void QIODevice::setCurrentReadChannel(int channel) void QIODevicePrivate::setReadChannelCount(int count) { if (count > readBuffers.size()) { + readBuffers.reserve(count); + // If readBufferChunkSize is zero, we should bypass QIODevice's // read buffers, even if the QIODeviceBase::Unbuffered flag is not // set when opened. However, if a read transaction is started or // ungetChar() is called, we still have to use the internal buffer. // To support these cases, pass a default value to the QRingBuffer // constructor. - readBuffers.insert(readBuffers.end(), count - readBuffers.size(), - QRingBuffer(readBufferChunkSize != 0 ? readBufferChunkSize - : QIODEVICE_BUFFERSIZE)); + + while (readBuffers.size() < count) + readBuffers.emplace_back(readBufferChunkSize != 0 ? readBufferChunkSize + : QIODEVICE_BUFFERSIZE); } else { readBuffers.resize(count); } @@ -762,8 +765,9 @@ void QIODevicePrivate::setWriteChannelCount(int count) // If writeBufferChunkSize is zero (default value), we don't use // QIODevice's write buffers. if (writeBufferChunkSize != 0) { - writeBuffers.insert(writeBuffers.end(), count - writeBuffers.size(), - QRingBuffer(writeBufferChunkSize)); + writeBuffers.reserve(count); + while (writeBuffers.size() < count) + writeBuffers.emplace_back(writeBufferChunkSize); } } else { writeBuffers.resize(count); diff --git a/src/corelib/tools/qringbuffer_p.h b/src/corelib/tools/qringbuffer_p.h index 0f5ce04c83..d01c31b861 100644 --- a/src/corelib/tools/qringbuffer_p.h +++ b/src/corelib/tools/qringbuffer_p.h @@ -189,10 +189,14 @@ Q_DECLARE_SHARED(QRingChunk) class QRingBuffer { + Q_DISABLE_COPY(QRingBuffer) public: explicit inline QRingBuffer(int growth = QRINGBUFFER_CHUNKSIZE) : bufferSize(0), basicBlockSize(growth) { } + QRingBuffer(QRingBuffer &&) noexcept = default; + QRingBuffer &operator=(QRingBuffer &&) noexcept = default; + inline void setChunkSize(int size) { basicBlockSize = size; } diff --git a/tests/auto/corelib/tools/qringbuffer/tst_qringbuffer.cpp b/tests/auto/corelib/tools/qringbuffer/tst_qringbuffer.cpp index 3b922de0ca..6efa39f4f5 100644 --- a/tests/auto/corelib/tools/qringbuffer/tst_qringbuffer.cpp +++ b/tests/auto/corelib/tools/qringbuffer/tst_qringbuffer.cpp @@ -38,6 +38,7 @@ class tst_QRingBuffer : public QObject private slots: void constructing(); void usingInVector(); + void usingInVarLengthArray(); void readPointerAtPositionWriteRead(); void readPointerAtPositionEmptyRead(); void readPointerAtPositionWithHead(); @@ -83,10 +84,20 @@ void tst_QRingBuffer::constructing() void tst_QRingBuffer::usingInVector() { QRingBuffer ringBuffer; - QList buffers; + std::vector buffers; ringBuffer.reserve(5); - buffers.append(ringBuffer); + buffers.push_back(std::move(ringBuffer)); + QCOMPARE(buffers[0].size(), Q_INT64_C(5)); +} + +void tst_QRingBuffer::usingInVarLengthArray() +{ + QRingBuffer ringBuffer; + QVarLengthArray buffers; + + ringBuffer.reserve(5); + buffers.push_back(std::move(ringBuffer)); QCOMPARE(buffers[0].size(), Q_INT64_C(5)); }