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, 42 insertions, 57 deletions

diff --git a/src/serialport/qserialport_wince.cpp b/src/serialport/qserialport_wince.cpp
index 04b00a77..cd28d254 100644
--- a/src/serialport/qserialport_wince.cpp
+++ b/src/serialport/qserialport_wince.cpp
@@ -178,8 +178,6 @@ private:
 
 bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
 {
-    Q_Q(QSerialPort);
-
     DWORD desiredAccess = 0;
     DWORD eventMask = EV_ERR;
 
@@ -196,7 +194,7 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
                               desiredAccess, 0, Q_NULLPTR, OPEN_EXISTING, 0, Q_NULLPTR);
 
     if (handle == INVALID_HANDLE_VALUE) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -227,12 +225,10 @@ void QSerialPortPrivate::close()
 
 QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals()
 {
-    Q_Q(QSerialPort);
-
     DWORD modemStat = 0;
 
     if (!::GetCommModemStatus(handle, &modemStat)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return QSerialPort::NoSignal;
     }
 
@@ -251,7 +247,7 @@ QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals()
     if (!::DeviceIoControl(handle, IOCTL_SERIAL_GET_DTRRTS, Q_NULLPTR, 0,
                           &modemStat, sizeof(modemStat),
                           &bytesReturned, Q_NULLPTR)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return ret;
     }
 
@@ -265,10 +261,8 @@ QSerialPort::PinoutSignals QSerialPortPrivate::pinoutSignals()
 
 bool QSerialPortPrivate::setDataTerminalReady(bool set)
 {
-    Q_Q(QSerialPort);
-
     if (!::EscapeCommFunction(handle, set ? SETDTR : CLRDTR)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -278,10 +272,8 @@ bool QSerialPortPrivate::setDataTerminalReady(bool set)
 
 bool QSerialPortPrivate::setRequestToSend(bool set)
 {
-    Q_Q(QSerialPort);
-
     if (!::EscapeCommFunction(handle, set ? SETRTS : CLRRTS)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -318,10 +310,8 @@ bool QSerialPortPrivate::sendBreak(int duration)
 
 bool QSerialPortPrivate::setBreakEnabled(bool set)
 {
-    Q_Q(QSerialPort);
-
     if (set ? !::SetCommBreak(handle) : !::ClearCommBreak(handle)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -389,10 +379,8 @@ bool QSerialPortPrivate::setBaudRate()
 
 bool QSerialPortPrivate::setBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
-    Q_Q(QSerialPort);
-
     if (directions != QSerialPort::AllDirections) {
-        q->setError(QSerialPort::UnsupportedOperationError);
+        setError(QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError));
         return false;
     }
     currentDcb.BaudRate = baudRate;
@@ -503,7 +491,7 @@ bool QSerialPortPrivate::notifyRead()
 
     if (!sucessResult) {
         buffer.chop(bytesToRead);
-        q->setError(QSerialPort::ReadError);
+        setError(QSerialPortErrorInfo(QSerialPort::ReadError));
         return false;
     }
 
@@ -546,7 +534,7 @@ bool QSerialPortPrivate::notifyWrite()
 
     DWORD bytesWritten = 0;
     if (!::WriteFile(handle, ptr, nextSize, &bytesWritten, Q_NULLPTR)) {
-        q->setError(QSerialPort::WriteError);
+        setError(QSerialPortErrorInfo(QSerialPort::WriteError));
         return false;
     }
 
@@ -566,31 +554,31 @@ qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize)
     return maxSize;
 }
 
-void QSerialPortPrivate::processIoErrors(bool error)
+void QSerialPortPrivate::processIoErrors(bool hasError)
 {
-    Q_Q(QSerialPort);
-
-    if (error) {
-        q->setError(QSerialPort::ResourceError);
+    if (hasError) {
+        setError(QSerialPortErrorInfo(QSerialPort::ResourceError));
         return;
     }
 
     DWORD errors = 0;
     if (!::ClearCommError(handle, &errors, Q_NULLPTR)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return;
     }
 
+    QSerialPortErrorInfo error;
+
     if (errors & CE_FRAME) {
-        q->setError(QSerialPort::FramingError);
+        error.errorCode = QSerialPort::FramingError;
     } else if (errors & CE_RXPARITY) {
-        q->setError(QSerialPort::ParityError);
+        error.errorCode = QSerialPort::ParityError;
         parityErrorOccurred = true;
     } else if (errors & CE_BREAK) {
-        q->setError(QSerialPort::BreakConditionError);
-    } else {
-        q->setError(QSerialPort::UnknownError);
+        error.errorCode = QSerialPort::BreakConditionError;
     }
+
+    setError(error);
 }
 
 inline bool QSerialPortPrivate::initialize(DWORD eventMask)
@@ -601,7 +589,7 @@ inline bool QSerialPortPrivate::initialize(DWORD eventMask)
     restoredDcb.DCBlength = sizeof(restoredDcb);
 
     if (!::GetCommState(handle, &restoredDcb)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -620,7 +608,7 @@ inline bool QSerialPortPrivate::initialize(DWORD eventMask)
         return false;
 
     if (!::GetCommTimeouts(handle, &restoredCommTimeouts)) {
-        q->setError(decodeSystemError());
+        setError(getSystemError());
         return false;
     }
 
@@ -638,8 +626,6 @@ inline bool QSerialPortPrivate::initialize(DWORD eventMask)
 
 bool QSerialPortPrivate::updateDcb()
 {
-    Q_Q(QSerialPort);
-
     QMutexLocker locker(&settingsChangeMutex);
 
     DWORD eventMask = 0;
@@ -652,7 +638,7 @@ bool QSerialPortPrivate::updateDcb()
     // Change parameters
     bool ret = ::SetCommState(handle, &currentDcb);
     if (!ret)
-        q->setError(decodeSystemError());
+        setError(getSystemError());
     // Restore the event mask
     ::SetCommMask(handle, eventMask);
 
@@ -661,50 +647,51 @@ bool QSerialPortPrivate::updateDcb()
 
 bool QSerialPortPrivate::updateCommTimeouts()
 {
-    Q_Q(QSerialPort);
-
     if (!::SetCommTimeouts(handle, &currentCommTimeouts)) {
-        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 = ::GetLastError();
 
-    QSerialPort::SerialPortError error;
-    switch (::GetLastError()) {
+    QSerialPortErrorInfo error;
+    error.errorString = qt_error_string(systemErrorCode);
+
+    switch (systemErrorCode) {
     case ERROR_IO_PENDING:
-        error = QSerialPort::NoError;
+        error.errorCode = QSerialPort::NoError;
         break;
     case ERROR_MORE_DATA:
-        error = QSerialPort::NoError;
+        error.errorCode = QSerialPort::NoError;
         break;
     case ERROR_FILE_NOT_FOUND:
-        error = QSerialPort::DeviceNotFoundError;
+        error.errorCode = QSerialPort::DeviceNotFoundError;
         break;
     case ERROR_INVALID_NAME:
-        error = QSerialPort::DeviceNotFoundError;
+        error.errorCode = QSerialPort::DeviceNotFoundError;
         break;
     case ERROR_ACCESS_DENIED:
-        error = QSerialPort::PermissionError;
+        error.errorCode = QSerialPort::PermissionError;
         break;
     case ERROR_INVALID_HANDLE:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
     case ERROR_INVALID_PARAMETER:
-        error = QSerialPort::UnsupportedOperationError;
+        error.errorCode = QSerialPort::UnsupportedOperationError;
         break;
     case ERROR_BAD_COMMAND:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
     case ERROR_DEVICE_REMOVED:
-        error = QSerialPort::ResourceError;
+        error.errorCode = QSerialPort::ResourceError;
         break;
     default:
-        error = QSerialPort::UnknownError;
+        error.errorCode = QSerialPort::UnknownError;
         break;
     }
     return error;
@@ -714,8 +701,6 @@ bool QSerialPortPrivate::waitForReadOrWrite(bool *selectForRead, bool *selectFor
                                            bool checkRead, bool checkWrite,
                                            int msecs)
 {
-    Q_Q(QSerialPort);
-
     DWORD eventMask = 0;
     // FIXME: Here the situation is not properly handled with zero timeout:
     // breaker can work out before you call a method WaitCommEvent()
@@ -725,7 +710,7 @@ bool QSerialPortPrivate::waitForReadOrWrite(bool *selectForRead, bool *selectFor
     breaker.stop();
 
     if (breaker.isWorked()) {
-        q->setError(QSerialPort::TimeoutError);
+        setError(QSerialPortErrorInfo(QSerialPort::TimeoutError));
     } else {
         if (checkRead) {
             Q_ASSERT(selectForRead);