From 18de39f7620ba4111f3846d4e8ea67aa5b701940 Mon Sep 17 00:00:00 2001 From: Laszlo Papp Date: Wed, 12 Feb 2014 13:54:23 +0000 Subject: Revert the private qtcore_unix related changes because they break the Qt 4 build Revert "Open file descriptors thread-safely on Unix and don't leak them" This reverts commit 691212d9e492b12590d5b7f4e2b24921911d4b17. Revert "Protect against EINTR in Unix non-atomic I/O calls" This reverts commit a8597cbae47076e33b638c2e593f852c8c0a02d5. Change-Id: I6d931ae6d3b4bee9be019fdf4a08ecc5a41a2b0f Reviewed-by: Sergey Belyashov --- src/serialport/qserialport_unix.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp index 84f3109..844ba13 100644 --- a/src/serialport/qserialport_unix.cpp +++ b/src/serialport/qserialport_unix.cpp @@ -59,8 +59,6 @@ #include #include -#include - QT_BEGIN_NAMESPACE QString serialPortLockFilePath(const QString &portName) @@ -215,7 +213,7 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode) break; } - descriptor = qt_safe_open(systemLocation.toLocal8Bit().constData(), flags); + descriptor = ::open(systemLocation.toLocal8Bit().constData(), flags); if (descriptor == -1) { q->setError(decodeSystemError()); @@ -306,7 +304,7 @@ void QSerialPortPrivate::close() exceptionNotifier = 0; } - if (qt_safe_close(descriptor) == -1) + if (::close(descriptor) == -1) q->setError(decodeSystemError()); if (lockFileScopedPointer->isLocked()) @@ -1078,7 +1076,7 @@ qint64 QSerialPortPrivate::readFromPort(char *data, qint64 maxSize) if (parity != QSerialPort::MarkParity && parity != QSerialPort::SpaceParity) { #endif - bytesRead = qt_safe_read(descriptor, data, maxSize); + bytesRead = ::read(descriptor, data, maxSize); } else {// Perform parity emulation. bytesRead = readPerChar(data, maxSize); } @@ -1090,11 +1088,11 @@ qint64 QSerialPortPrivate::writeToPort(const char *data, qint64 maxSize) { qint64 bytesWritten = 0; #if defined (CMSPAR) - bytesWritten = qt_safe_write(descriptor, data, maxSize); + bytesWritten = ::write(descriptor, data, maxSize); #else if (parity != QSerialPort::MarkParity && parity != QSerialPort::SpaceParity) { - bytesWritten = qt_safe_write(descriptor, data, maxSize); + bytesWritten = ::write(descriptor, data, maxSize); } else {// Perform parity emulation. bytesWritten = writePerChar(data, maxSize); } @@ -1131,7 +1129,7 @@ qint64 QSerialPortPrivate::writePerChar(const char *data, qint64 maxSize) break; } - int r = qt_safe_write(descriptor, data, 1); + int r = ::write(descriptor, data, 1); if (r < 0) return -1; if (r > 0) { @@ -1157,7 +1155,7 @@ qint64 QSerialPortPrivate::readPerChar(char *data, qint64 maxSize) int prefix = 0; while (ret < maxSize) { - qint64 r = qt_safe_read(descriptor, data, 1); + qint64 r = ::read(descriptor, data, 1); if (r < 0) { if (errno == EAGAIN) // It is ok for nonblocking mode. break; -- cgit v1.2.1 From 21f50498f7558e59bca8789e94c0fe5d14a9e27d Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Wed, 22 Jan 2014 17:52:16 +0400 Subject: Do not emit bytesWritten() while data aren't transferred The serial port very slow device therefore the system call write() doesn't guarantee that data were transferred completely to the device. It guarantee only that data were sent to the driver from the user space. Thus it is wrong to emit the bytesWritten() signal at once after successful writing. The correct solution is to store the number of transferred data in some variable (e.g. pendingBytesWritten in this case) and to wait for the next notification from the WriteNotifier that the write FIFO of device is empty (i.e. when all data were really transferred) and ready to transmission of the next portion of data. Also good decision is to divide a data transfer operation into two methods: startAsyncWrite() and completeAsyncWrite(), similar with the Windows implementation. Where the startAsyncWrite() invokes the write() system call, and the completeAsyncWrite() is invoked by a writeNotifier to complete writing operation or start a new startAsyncWrite() operation. Tested on Arch Linux 64 bit with the on-board and the USB (PL2303) serial ports with use Qt4 and then Qt5. Change-Id: I1c274b2052a9bd54811586c6f1cfdf080b400263 Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialport_unix.cpp | 57 ++++++++++++++++++++++--------------- src/serialport/qserialport_unix_p.h | 6 +++- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp index 844ba13..d99575f 100644 --- a/src/serialport/qserialport_unix.cpp +++ b/src/serialport/qserialport_unix.cpp @@ -133,7 +133,7 @@ protected: bool event(QEvent *e) Q_DECL_OVERRIDE { bool ret = QSocketNotifier::event(e); if (ret) - dptr->writeNotification(); + dptr->completeAsyncWrite(); return ret; } @@ -176,6 +176,8 @@ QSerialPortPrivate::QSerialPortPrivate(QSerialPort *q) , readPortNotifierStateSet(false) , emittedReadyRead(false) , emittedBytesWritten(false) + , pendingBytesWritten(0) + , writeSequenceStarted(false) { } @@ -312,6 +314,8 @@ void QSerialPortPrivate::close() descriptor = -1; isCustomBaudRateSupported = false; + pendingBytesWritten = 0; + writeSequenceStarted = false; } QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals() @@ -401,7 +405,7 @@ bool QSerialPortPrivate::setRequestToSend(bool set) bool QSerialPortPrivate::flush() { - return writeNotification() + return startAsyncWrite() #ifndef Q_OS_ANDROID && (::tcdrain(descriptor) != -1); #else @@ -477,7 +481,7 @@ bool QSerialPortPrivate::waitForReadyRead(int msecs) } if (readyToWrite) - writeNotification(); + completeAsyncWrite(); } while (msecs == -1 || timeoutValue(msecs, stopWatch.elapsed()) > 0); return false; @@ -487,7 +491,7 @@ bool QSerialPortPrivate::waitForBytesWritten(int msecs) { Q_Q(QSerialPort); - if (writeBuffer.isEmpty()) + if (writeBuffer.isEmpty() && pendingBytesWritten <= 0) return false; QElapsedTimer stopWatch; @@ -509,7 +513,7 @@ bool QSerialPortPrivate::waitForBytesWritten(int msecs) return false; if (readyToWrite) - return writeNotification(); + return completeAsyncWrite(); } return false; } @@ -788,23 +792,15 @@ bool QSerialPortPrivate::readNotification() return true; } -bool QSerialPortPrivate::writeNotification() +bool QSerialPortPrivate::startAsyncWrite() { Q_Q(QSerialPort); - const int tmp = writeBuffer.size(); - - if (writeBuffer.isEmpty()) { - setWriteNotificationEnabled(false); - return false; - } - - int nextSize = writeBuffer.nextDataBlockSize(); - - const char *ptr = writeBuffer.readPointer(); + if (writeBuffer.isEmpty() || writeSequenceStarted) + return true; // Attempt to write it all in one chunk. - qint64 written = writeToPort(ptr, nextSize); + qint64 written = writeToPort(writeBuffer.readPointer(), writeBuffer.nextDataBlockSize()); if (written < 0) { QSerialPort::SerialPortError error = decodeSystemError(); if (error != QSerialPort::ResourceError) @@ -813,21 +809,36 @@ bool QSerialPortPrivate::writeNotification() return false; } - // Remove what we wrote so far. writeBuffer.free(written); - if (written > 0) { - // Don't emit bytesWritten() recursively. + pendingBytesWritten += written; + writeSequenceStarted = true; + + if (!isWriteNotificationEnabled()) + setWriteNotificationEnabled(true); + return true; +} + +bool QSerialPortPrivate::completeAsyncWrite() +{ + Q_Q(QSerialPort); + + if (pendingBytesWritten > 0) { if (!emittedBytesWritten) { emittedBytesWritten = true; - emit q->bytesWritten(written); + emit q->bytesWritten(pendingBytesWritten); + pendingBytesWritten = 0; emittedBytesWritten = false; } } - if (writeBuffer.isEmpty()) + writeSequenceStarted = false; + + if (writeBuffer.isEmpty()) { setWriteNotificationEnabled(false); + return true; + } - return (writeBuffer.size() < tmp); + return startAsyncWrite(); } void QSerialPortPrivate::exceptionNotification() diff --git a/src/serialport/qserialport_unix_p.h b/src/serialport/qserialport_unix_p.h index 1960316..ed86fc8 100644 --- a/src/serialport/qserialport_unix_p.h +++ b/src/serialport/qserialport_unix_p.h @@ -121,7 +121,8 @@ public: bool setDataErrorPolicy(QSerialPort::DataErrorPolicy policy); bool readNotification(); - bool writeNotification(); + bool startAsyncWrite(); + bool completeAsyncWrite(); void exceptionNotification(); static QString portNameToSystemLocation(const QString &port); @@ -152,6 +153,9 @@ public: bool emittedReadyRead; bool emittedBytesWritten; + qint64 pendingBytesWritten; + bool writeSequenceStarted; + QScopedPointer lockFileScopedPointer; private: -- cgit v1.2.1