Improve the processing of errors

Sometimes the error string would contain a wrong description which
did not correspond to the system error code. The reason was
qt_error_string() being called too late, when the system error
might have already been overwritten.

The error processing is now in QSPP::getSystemError(), which returns both
the error code and the error description as soon as possible.

* Now the QSPP::getSystemError() returns the new class
  QSerialPortErrorInfo which contains all necessary fields.

* The new method QSPP::setError() which accepts the
  QSerialPortErrorInfo as input parameter is used.

* The old private method QSP::setError() is removed, because
  it is not used anywhere.

Change-Id: Ia7e4d617b863e2131175c52812cdf426ed963795
Reviewed-by: Sergey Belyashov <Sergey.Belyashov@gmail.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Reviewed-by: Denis Shienkov <denis.shienkov@gmail.com>

1 files changed, 74 insertions, 96 deletions

diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp
index d2c0eca..a80d6b3 100644
--- a/src/serialport/qserialport_unix.cpp
+++ b/src/serialport/qserialport_unix.cpp
@@ -158,20 +158,18 @@ private:
 
 bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
 {
-    Q_Q(QSerialPort);
-
     QString lockFilePath = serialPortLockFilePath(QSerialPortInfoPrivate::portNameFromSystemLocation(systemLocation));
     bool isLockFileEmpty = lockFilePath.isEmpty();
     if (isLockFileEmpty) {
         qWarning("Failed to create a lock file for opening the device");
-        q->setError(QSerialPort::PermissionError);
+        setError(QSerialPortErrorInfo(QSerialPort::PermissionError));
         return false;
     }
 
     QScopedPointer<QLockFile> newLockFileScopedPointer(new QLockFile(lockFilePath));
 
     if (!newLockFileScopedPointer->tryLock()) {
-        q->setError(QSerialPort::PermissionError);
+        setError(QSerialPortErrorInfo(QSerialPort::PermissionError));
         return false;
     }
 
@@ -192,7 +190,7 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
     descriptor = qt_safe_open(systemLocation.toLocal8Bit().constData(), flags);
 
     if (descriptor == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -208,16 +206,14 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
 
 void QSerialPortPrivate::close()
 {
-    Q_Q(QSerialPort);
-
     if (settingsRestoredOnClose) {
         if (::tcsetattr(descriptor, TCSANOW, &restoredTermios) == -1)
-            q->setError(decodeSystemError());
+            setError(getSystemError());
     }
 
 #ifdef TIOCNXCL
     if (::ioctl(descriptor, TIOCNXCL) == -1)
-        q->setError(decodeSystemError());
+        setError(getSystemError());
 #endif
 
     if (readNotifier) {
@@ -233,7 +229,7 @@ void QSerialPortPrivate::close()
     }
 
     if (qt_safe_close(descriptor) == -1)
-        q->setError(decodeSystemError());
+        setError(getSystemError());
 
     lockFileScopedPointer.reset(Q_NULLPTR);
 
@@ -244,12 +240,10 @@ void QSerialPortPrivate::close()
 
 QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals()
 {
-    Q_Q(QSerialPort);
-
     int arg = 0;
 
     if (::ioctl(descriptor, TIOCMGET, &arg) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return QSerialPort::NoSignal;
     }
 
@@ -303,11 +297,9 @@ QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals()
 
 bool QSerialPortPrivate::setDataTerminalReady(bool set)
 {
-    Q_Q(QSerialPort);
-
     int status = TIOCM_DTR;
     if (::ioctl(descriptor, set ? TIOCMBIS : TIOCMBIC, &status) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -316,11 +308,9 @@ bool QSerialPortPrivate::setDataTerminalReady(bool set)
 
 bool QSerialPortPrivate::setRequestToSend(bool set)
 {
-    Q_Q(QSerialPort);
-
     int status = TIOCM_RTS;
     if (::ioctl(descriptor, set ? TIOCMBIS : TIOCMBIC, &status) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -334,11 +324,9 @@ bool QSerialPortPrivate::flush()
 
 bool QSerialPortPrivate::clear(QSerialPort::Directions directions)
 {
-    Q_Q(QSerialPort);
-
     if (::tcflush(descriptor, (directions == QSerialPort::AllDirections)
                      ? TCIOFLUSH : (directions & QSerialPort::Input) ? TCIFLUSH : TCOFLUSH) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -347,10 +335,8 @@ bool QSerialPortPrivate::clear(QSerialPort::Directions directions)
 
 bool QSerialPortPrivate::sendBreak(int duration)
 {
-    Q_Q(QSerialPort);
-
     if (::tcsendbreak(descriptor, duration) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -359,10 +345,8 @@ bool QSerialPortPrivate::sendBreak(int duration)
 
 bool QSerialPortPrivate::setBreakEnabled(bool set)
 {
-    Q_Q(QSerialPort);
-
     if (::ioctl(descriptor, set ? TIOCSBRK : TIOCCBRK) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -425,22 +409,22 @@ bool QSerialPortPrivate::setBaudRate()
         && setBaudRate(outputBaudRate, QSerialPort::Output));
 }
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setBaudRate_helper(qint32 baudRate,
         QSerialPort::Directions directions)
 {
     if ((directions & QSerialPort::Input) && ::cfsetispeed(&currentTermios, baudRate) < 0)
-            return decodeSystemError();
+        return getSystemError();
 
     if ((directions & QSerialPort::Output) && ::cfsetospeed(&currentTermios, baudRate) < 0)
-            return decodeSystemError();
+        return getSystemError();
 
-    return QSerialPort::NoError;
+    return QSerialPortErrorInfo(QSerialPort::NoError);
 }
 
 #if defined(Q_OS_LINUX)
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     struct serial_struct currentSerialInfo;
@@ -452,7 +436,7 @@ QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions
         currentSerialInfo.flags &= ~ASYNC_SPD_CUST;
         currentSerialInfo.custom_divisor = 0;
         if (::ioctl(descriptor, TIOCSSERIAL, &currentSerialInfo) == -1)
-            return decodeSystemError();
+            return getSystemError();
     }
 
     return setBaudRate_helper(baudRate, directions);
@@ -460,7 +444,7 @@ QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions
 
 #else
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     return setBaudRate_helper(baudRate, directions);
@@ -470,7 +454,7 @@ QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions
 
 #if defined(Q_OS_LINUX)
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     Q_UNUSED(directions);
@@ -480,14 +464,14 @@ QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions d
     ::memset(&currentSerialInfo, 0, sizeof(currentSerialInfo));
 
     if (::ioctl(descriptor, TIOCGSERIAL, &currentSerialInfo) == -1)
-        return decodeSystemError();
+        return getSystemError();
 
     currentSerialInfo.flags &= ~ASYNC_SPD_MASK;
     currentSerialInfo.flags |= (ASYNC_SPD_CUST /* | ASYNC_LOW_LATENCY*/);
     currentSerialInfo.custom_divisor = currentSerialInfo.baud_base / baudRate;
 
     if (currentSerialInfo.custom_divisor == 0)
-        return QSerialPort::UnsupportedOperationError;
+        return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError);
 
     if (currentSerialInfo.custom_divisor * baudRate != currentSerialInfo.baud_base) {
         qWarning("Baud rate of serial port %s is set to %d instead of %d: divisor %f unsupported",
@@ -497,31 +481,31 @@ QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions d
     }
 
     if (::ioctl(descriptor, TIOCSSERIAL, &currentSerialInfo) == -1)
-        return decodeSystemError();
+        return getSystemError();
 
     return setBaudRate_helper(B38400, directions);
 }
 
 #elif defined(Q_OS_MAC)
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     Q_UNUSED(directions);
 
 #if defined(MAC_OS_X_VERSION_10_4) && (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_4)
     if (::ioctl(descriptor, IOSSIOSPEED, &baudRate) == -1)
-        return decodeSystemError();
+        return getSystemError();
 
-    return QSerialPort::NoError;
+    return QSerialPortErrorInfo(QSerialPort::NoError);
 #endif
 
-    return QSerialPort::UnsupportedOperationError;
+    return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError);
 }
 
 #elif defined(Q_OS_QNX)
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     // On QNX, the values of the 'Bxxxx' constants are set to 'xxxx' (i.e.
@@ -533,36 +517,34 @@ QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions d
 
 #else
 
-QSerialPort::SerialPortError
+QSerialPortErrorInfo
 QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     Q_UNUSED(baudRate);
     Q_UNUSED(directions);
 
-    return QSerialPort::UnsupportedOperationError;
+    return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError);
 }
 
 #endif
 
 bool QSerialPortPrivate::setBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
-    Q_Q(QSerialPort);
-
     if (baudRate <= 0) {
-        q->setError(QSerialPort::UnsupportedOperationError);
+        setError(QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError));
         return false;
     }
 
     const qint32 unixBaudRate = QSerialPortPrivate::settingFromBaudRate(baudRate);
 
-    const QSerialPort::SerialPortError error = (unixBaudRate > 0)
+    const QSerialPortErrorInfo error = (unixBaudRate > 0)
         ? setStandardBaudRate(unixBaudRate, directions)
         : setCustomBaudRate(baudRate, directions);
 
-    if (error == QSerialPort::NoError)
+    if (error.errorCode == QSerialPort::NoError)
         return updateTermios();
 
-    q->setError(error);
+    setError(error);
     return false;
 }
 
@@ -719,12 +701,12 @@ bool QSerialPortPrivate::readNotification()
     const qint64 readBytes = readFromPort(ptr, bytesToRead);
 
     if (readBytes <= 0) {
-        QSerialPort::SerialPortError error = decodeSystemError();
-        if (error != QSerialPort::ResourceError)
-            error = QSerialPort::ReadError;
+        QSerialPortErrorInfo error = getSystemError();
+        if (error.errorCode != QSerialPort::ResourceError)
+            error.errorCode = QSerialPort::ReadError;
         else
             setReadNotificationEnabled(false);
-        q->setError(error);
+        setError(error);
         buffer.chop(bytesToRead);
         return false;
     }
@@ -751,18 +733,16 @@ bool QSerialPortPrivate::readNotification()
 
 bool QSerialPortPrivate::startAsyncWrite()
 {
-    Q_Q(QSerialPort);
-
     if (writeBuffer.isEmpty() || writeSequenceStarted)
         return true;
 
     // Attempt to write it all in one chunk.
     qint64 written = writeToPort(writeBuffer.readPointer(), writeBuffer.nextDataBlockSize());
     if (written < 0) {
-        QSerialPort::SerialPortError error = decodeSystemError();
-        if (error != QSerialPort::ResourceError)
-            error = QSerialPort::WriteError;
-        q->setError(error);
+        QSerialPortErrorInfo error = getSystemError();
+        if (error.errorCode != QSerialPort::ResourceError)
+            error.errorCode = QSerialPort::WriteError;
+        setError(error);
         return false;
     }
 
@@ -800,15 +780,13 @@ bool QSerialPortPrivate::completeAsyncWrite()
 
 inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode)
 {
-    Q_Q(QSerialPort);
-
 #ifdef TIOCEXCL
     if (::ioctl(descriptor, TIOCEXCL) == -1)
-        q->setError(decodeSystemError());
+        setError(getSystemError());
 #endif
 
     if (::tcgetattr(descriptor, &restoredTermios) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -848,71 +826,72 @@ qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize)
 
 bool QSerialPortPrivate::updateTermios()
 {
-    Q_Q(QSerialPort);
-
     if (::tcsetattr(descriptor, TCSANOW, &currentTermios) == -1) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
     return true;
 }
 
-QSerialPort::SerialPortError QSerialPortPrivate::decodeSystemError(int systemErrorCode) const
+QSerialPortErrorInfo QSerialPortPrivate::getSystemError(int systemErrorCode) const
 {
-    Q_UNUSED(systemErrorCode);
+    if (systemErrorCode == -1)
+        systemErrorCode = errno;
+
+    QSerialPortErrorInfo error;
+    error.errorString = qt_error_string(systemErrorCode);
 
-    QSerialPort::SerialPortError error;
-    switch (errno) {
+    switch (systemErrorCode) {
     case ENODEV:
-        error = QSerialPort::DeviceNotFoundError;
+        error.errorCode = QSerialPort::DeviceNotFoundError;
         break;
 #ifdef ENOENT
     case ENOENT:
-        error = QSerialPort::DeviceNotFoundError;
+        error.errorCode = QSerialPort::DeviceNotFoundError;
         break;
 #endif
     case EACCES:
-        error = QSerialPort::PermissionError;
+        error.errorCode = QSerialPort::PermissionError;
         break;
     case EBUSY:
-        error = QSerialPort::PermissionError;
+        error.errorCode = QSerialPort::PermissionError;
         break;
     case EAGAIN:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
     case EIO:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
     case EBADF:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
 #ifdef Q_OS_MAC
     case ENXIO:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
 #endif
 #ifdef EINVAL
     case EINVAL:
-        error = QSerialPort::UnsupportedOperationError;
+        error.errorCode = QSerialPort::UnsupportedOperationError;
         break;
 #endif
 #ifdef ENOIOCTLCMD
     case ENOIOCTLCMD:
-        error = QSerialPort::UnsupportedOperationError;
+        error.errorCode = QSerialPort::UnsupportedOperationError;
         break;
 #endif
 #ifdef ENOTTY
     case ENOTTY:
-        error = QSerialPort::UnsupportedOperationError;
+        error.errorCode = QSerialPort::UnsupportedOperationError;
         break;
 #endif
 #ifdef EPERM
     case EPERM:
-        error = QSerialPort::PermissionError;
+        error.errorCode = QSerialPort::PermissionError;
         break;
 #endif
     default:
-        error = QSerialPort::UnknownError;
+        error.errorCode = QSerialPort::UnknownError;
         break;
     }
     return error;
@@ -956,8 +935,6 @@ bool QSerialPortPrivate::waitForReadOrWrite(bool *selectForRead, bool *selectFor
                                            bool checkRead, bool checkWrite,
                                            int msecs)
 {
-    Q_Q(QSerialPort);
-
     Q_ASSERT(selectForRead);
     Q_ASSERT(selectForWrite);
 
@@ -977,11 +954,11 @@ bool QSerialPortPrivate::waitForReadOrWrite(bool *selectForRead, bool *selectFor
 
     const int ret = ::select(descriptor + 1, &fdread, &fdwrite, 0, msecs < 0 ? 0 : &tv);
     if (ret < 0) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
     if (ret == 0) {
-        q->setError(QSerialPort::TimeoutError);
+        setError(QSerialPortErrorInfo(QSerialPort::TimeoutError));
         return false;
     }
 
@@ -1068,8 +1045,6 @@ qint64 QSerialPortPrivate::writePerChar(const char *data, qint64 maxSize)
 
 qint64 QSerialPortPrivate::readPerChar(char *data, qint64 maxSize)
 {
-    Q_Q(QSerialPort);
-
     qint64 ret = 0;
     quint8 const charMask = (0xFF >> (8 - dataBits));
 
@@ -1115,13 +1090,16 @@ qint64 QSerialPortPrivate::readPerChar(char *data, qint64 maxSize)
             switch (policy) {
             case QSerialPort::SkipPolicy:
                 continue;       //ignore received character
-            case QSerialPort::StopReceivingPolicy:
+            case QSerialPort::StopReceivingPolicy: {
+                QSerialPortErrorInfo error;
                 if (parity != QSerialPort::NoParity)
-                    q->setError(QSerialPort::ParityError);
+                    error.errorCode = QSerialPort::ParityError;
                 else
-                    q->setError(*data == '\0' ?
-                                QSerialPort::BreakConditionError : QSerialPort::FramingError);
+                    error.errorCode = *data == '\0' ?
+                                QSerialPort::BreakConditionError : QSerialPort::FramingError;
+                setError(error);
                 return ++ret;   //abort receiving
+            }
                 break;
             case QSerialPort::UnknownPolicy:
                 // Unknown error policy is used! Falling back to PassZeroPolicy