Detach buffer's segment for asynchronous write operation

QRingBuffer does not guarantee that the read pointer which is
returned by QRingBuffer::readPointer() will stay valid after
calling QRingBuffer::reserve() or QRingBuffer::append().

Thus, we need to detach the buffer's segment and pass it to the
WriteFile() function to ensure it stays unmodified for the whole
time of the asynchronous operation.

Change-Id: I5a42086b4339a050a4df57c39348864502be2c74
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Reviewed-by: Alex Trotsenko <alex1973tr@gmail.com>
Reviewed-by: Sergey Belyashov <Sergey.Belyashov@gmail.com>
Reviewed-by: Anton Kudryavtsev <a.kudryavtsev@netris.ru>
Reviewed-by: Denis Shienkov <denis.shienkov@gmail.com>

3 files changed, 11 insertions, 22 deletions

diff --git a/src/serialport/qserialport.cpp b/src/serialport/qserialport.cpp
index 481b07e..8b4449b 100644
--- a/src/serialport/qserialport.cpp
+++ b/src/serialport/qserialport.cpp
@@ -107,7 +107,6 @@ QSerialPortPrivate::QSerialPortPrivate()
     , startAsyncWriteTimer(0)
     , originalEventMask(0)
     , triggeredEventMask(0)
-    , actualBytesToWrite(0)
 #elif defined(Q_OS_UNIX)
     , descriptor(-1)
     , readNotifier(0)
@@ -1260,13 +1259,7 @@ qint64 QSerialPort::bytesAvailable() const
 qint64 QSerialPort::bytesToWrite() const
 {
     Q_D(const QSerialPort);
-    qint64 bytes = QIODevice::bytesToWrite();
-#ifdef Q_OS_WIN32
-    bytes += d->actualBytesToWrite;
-#else
-    bytes += d->writeBuffer.size();
-#endif
-    return bytes;
+    return QIODevice::bytesToWrite() + d->writeBuffer.size();
 }
 
 /*!
diff --git a/src/serialport/qserialport_p.h b/src/serialport/qserialport_p.h
index 81febed..567ca19 100644
--- a/src/serialport/qserialport_p.h
+++ b/src/serialport/qserialport_p.h
@@ -224,6 +224,7 @@ public:
     COMMTIMEOUTS restoredCommTimeouts;
     HANDLE handle;
     QByteArray readChunkBuffer;
+    QByteArray writeChunkBuffer;
     bool communicationStarted;
     bool writeStarted;
     bool readStarted;
@@ -234,7 +235,6 @@ public:
     OVERLAPPED writeCompletionOverlapped;
     DWORD originalEventMask;
     DWORD triggeredEventMask;
-    qint64 actualBytesToWrite;
 
 #elif defined(Q_OS_UNIX)
 
diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp
index 1680d5a..056cc31 100644
--- a/src/serialport/qserialport_win.cpp
+++ b/src/serialport/qserialport_win.cpp
@@ -116,7 +116,6 @@ void QSerialPortPrivate::close()
     readStarted = false;
     writeStarted = false;
     writeBuffer.clear();
-    actualBytesToWrite = 0;
 
     if (settingsRestoredOnClose) {
         ::SetCommState(handle, &restoredDcb);
@@ -198,10 +197,8 @@ bool QSerialPortPrivate::clear(QSerialPort::Directions directions)
     DWORD flags = 0;
     if (directions & QSerialPort::Input)
         flags |= PURGE_RXABORT | PURGE_RXCLEAR;
-    if (directions & QSerialPort::Output) {
+    if (directions & QSerialPort::Output)
         flags |= PURGE_TXABORT | PURGE_TXCLEAR;
-        actualBytesToWrite = 0;
-    }
     if (!::PurgeComm(handle, flags)) {
         setError(getSystemError());
         return false;
@@ -275,7 +272,7 @@ bool QSerialPortPrivate::waitForReadyRead(int msecs)
 
 bool QSerialPortPrivate::waitForBytesWritten(int msecs)
 {
-    if (writeBuffer.isEmpty())
+    if (writeBuffer.isEmpty() && writeChunkBuffer.isEmpty())
         return false;
 
     if (!writeStarted && !_q_startAsyncWrite())
@@ -453,12 +450,13 @@ bool QSerialPortPrivate::completeAsyncWrite(qint64 bytesTransferred)
 
     if (writeStarted) {
         if (bytesTransferred == qint64(-1)) {
+            writeChunkBuffer.clear();
             writeStarted = false;
             return false;
-        } else if (bytesTransferred > 0) {
-            writeBuffer.free(bytesTransferred);
-            emit q->bytesWritten(bytesTransferred);
         }
+        Q_ASSERT(bytesTransferred == writeChunkBuffer.size());
+        writeChunkBuffer.clear();
+        emit q->bytesWritten(bytesTransferred);
         writeStarted = false;
     }
 
@@ -525,10 +523,10 @@ bool QSerialPortPrivate::_q_startAsyncWrite()
     if (writeBuffer.isEmpty() || writeStarted)
         return true;
 
-    const int writeBytes = writeBuffer.nextDataBlockSize();
+    writeChunkBuffer = writeBuffer.read();
     ::ZeroMemory(&writeCompletionOverlapped, sizeof(writeCompletionOverlapped));
-    if (!::WriteFile(handle, writeBuffer.readPointer(),
-                     writeBytes, Q_NULLPTR, &writeCompletionOverlapped)) {
+    if (!::WriteFile(handle, writeChunkBuffer.constData(),
+                     writeChunkBuffer.size(), Q_NULLPTR, &writeCompletionOverlapped)) {
 
         QSerialPortErrorInfo error = getSystemError();
         if (error.errorCode != QSerialPort::NoError) {
@@ -539,7 +537,6 @@ bool QSerialPortPrivate::_q_startAsyncWrite()
         }
     }
 
-    actualBytesToWrite -= writeBytes;
     writeStarted = true;
     return true;
 }
@@ -574,7 +571,6 @@ qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize)
     Q_Q(QSerialPort);
 
     ::memcpy(writeBuffer.reserve(maxSize), data, maxSize);
-    actualBytesToWrite += maxSize;
 
     if (!writeBuffer.isEmpty() && !writeStarted) {
         if (!startAsyncWriteTimer) {