Fix leak of a descriptor after unsuccessful opening

In case of unsuccessful initialization of a device at the opening, a
valid descriptor has to be closed before return from the open() method.

Task-number: QTBUG-40414
Change-Id: I45568f176e003d9be1fe8c3017da29f39908efb0
Reviewed-by: Sergey Belyashov <Sergey.Belyashov@gmail.com>

6 files changed, 152 insertions, 115 deletions

diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp
index 29822d5..11ab16f 100644
--- a/src/serialport/qserialport_unix.cpp
+++ b/src/serialport/qserialport_unix.cpp
@@ -204,39 +204,11 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
         return false;
     }
 
-#ifdef TIOCEXCL
-    if (::ioctl(descriptor, TIOCEXCL) == -1)
-        q->setError(decodeSystemError());
-#endif
-
-    if (::tcgetattr(descriptor, &restoredTermios) == -1) {
-        q->setError(decodeSystemError());
+    if (!initialize(mode)) {
+        qt_safe_close(descriptor);
         return false;
     }
 
-    currentTermios = restoredTermios;
-#ifdef Q_OS_SOLARIS
-    currentTermios.c_iflag &= ~(IMAXBEL|IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
-    currentTermios.c_oflag &= ~OPOST;
-    currentTermios.c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
-    currentTermios.c_cflag &= ~(CSIZE|PARENB);
-    currentTermios.c_cflag |= CS8;
-#else
-    ::cfmakeraw(&currentTermios);
-#endif
-    currentTermios.c_cflag |= CLOCAL;
-    currentTermios.c_cc[VTIME] = 0;
-    currentTermios.c_cc[VMIN] = 0;
-
-    if (mode & QIODevice::ReadOnly)
-        currentTermios.c_cflag |= CREAD;
-
-    if (!updateTermios())
-        return false;
-
-    if ((flags & O_WRONLY) == 0)
-        setReadNotificationEnabled(true);
-
     lockFileScopedPointer.swap(newLockFileScopedPointer);
 
     return true;
@@ -869,6 +841,46 @@ bool QSerialPortPrivate::completeAsyncWrite()
     return startAsyncWrite();
 }
 
+inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode)
+{
+    Q_Q(QSerialPort);
+
+#ifdef TIOCEXCL
+    if (::ioctl(descriptor, TIOCEXCL) == -1)
+        q->setError(decodeSystemError());
+#endif
+
+    if (::tcgetattr(descriptor, &restoredTermios) == -1) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    currentTermios = restoredTermios;
+#ifdef Q_OS_SOLARIS
+    currentTermios.c_iflag &= ~(IMAXBEL|IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
+    currentTermios.c_oflag &= ~OPOST;
+    currentTermios.c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
+    currentTermios.c_cflag &= ~(CSIZE|PARENB);
+    currentTermios.c_cflag |= CS8;
+#else
+    ::cfmakeraw(&currentTermios);
+#endif
+    currentTermios.c_cflag |= CLOCAL;
+    currentTermios.c_cc[VTIME] = 0;
+    currentTermios.c_cc[VMIN] = 0;
+
+    if (mode & QIODevice::ReadOnly)
+        currentTermios.c_cflag |= CREAD;
+
+    if (!updateTermios())
+        return false;
+
+    if (mode & QIODevice::ReadOnly)
+        setReadNotificationEnabled(true);
+
+    return true;
+}
+
 bool QSerialPortPrivate::updateTermios()
 {
     Q_Q(QSerialPort);
diff --git a/src/serialport/qserialport_unix_p.h b/src/serialport/qserialport_unix_p.h
index 64d1d9e..ee3d82d 100644
--- a/src/serialport/qserialport_unix_p.h
+++ b/src/serialport/qserialport_unix_p.h
@@ -153,6 +153,7 @@ public:
     QScopedPointer<QLockFile> lockFileScopedPointer;
 
 private:
+    bool initialize(QIODevice::OpenMode mode);
     bool updateTermios();
 
     QSerialPort::SerialPortError setBaudRate_helper(qint32 baudRate,
diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp
index a6e1df7..eccaca9 100644
--- a/src/serialport/qserialport_win.cpp
+++ b/src/serialport/qserialport_win.cpp
@@ -155,56 +155,11 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
         return false;
     }
 
-    ::ZeroMemory(&restoredDcb, sizeof(restoredDcb));
-    restoredDcb.DCBlength = sizeof(restoredDcb);
-
-    if (!::GetCommState(handle, &restoredDcb)) {
-        q->setError(decodeSystemError());
-        return false;
-    }
-
-    currentDcb = restoredDcb;
-    currentDcb.fBinary = TRUE;
-    currentDcb.fInX = FALSE;
-    currentDcb.fOutX = FALSE;
-    currentDcb.fAbortOnError = FALSE;
-    currentDcb.fNull = FALSE;
-    currentDcb.fErrorChar = FALSE;
-
-    if (currentDcb.fDtrControl ==  DTR_CONTROL_HANDSHAKE)
-        currentDcb.fDtrControl = DTR_CONTROL_DISABLE;
-
-    if (!updateDcb())
-        return false;
-
-    if (!::GetCommTimeouts(handle, &restoredCommTimeouts)) {
-        q->setError(decodeSystemError());
-        return false;
-    }
-
-    ::ZeroMemory(&currentCommTimeouts, sizeof(currentCommTimeouts));
-    currentCommTimeouts.ReadIntervalTimeout = MAXDWORD;
-
-    if (!updateCommTimeouts())
-        return false;
-
-    if (mode & QIODevice::ReadOnly)
-        readCompletionNotifier->setEnabled(true);
-
-    if (mode & QIODevice::WriteOnly)
-        writeCompletionNotifier->setEnabled(true);
-
-    if (!::SetCommMask(handle, originalEventMask)) {
-        q->setError(decodeSystemError());
-        return false;
-    }
-
-    if (!startAsyncCommunication())
-        return false;
-
-    communicationNotifier->setEnabled(true);
+    if (initialize(mode))
+        return true;
 
-    return true;
+    ::CloseHandle(handle);
+    return false;
 }
 
 void QSerialPortPrivate::close()
@@ -743,6 +698,62 @@ void QSerialPortPrivate::handleLineStatusErrors()
     }
 }
 
+inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode)
+{
+    Q_Q(QSerialPort);
+
+    ::ZeroMemory(&restoredDcb, sizeof(restoredDcb));
+    restoredDcb.DCBlength = sizeof(restoredDcb);
+
+    if (!::GetCommState(handle, &restoredDcb)) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    currentDcb = restoredDcb;
+    currentDcb.fBinary = TRUE;
+    currentDcb.fInX = FALSE;
+    currentDcb.fOutX = FALSE;
+    currentDcb.fAbortOnError = FALSE;
+    currentDcb.fNull = FALSE;
+    currentDcb.fErrorChar = FALSE;
+
+    if (currentDcb.fDtrControl ==  DTR_CONTROL_HANDSHAKE)
+        currentDcb.fDtrControl = DTR_CONTROL_DISABLE;
+
+    if (!updateDcb())
+        return false;
+
+    if (!::GetCommTimeouts(handle, &restoredCommTimeouts)) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    ::ZeroMemory(&currentCommTimeouts, sizeof(currentCommTimeouts));
+    currentCommTimeouts.ReadIntervalTimeout = MAXDWORD;
+
+    if (!updateCommTimeouts())
+        return false;
+
+    if (mode & QIODevice::ReadOnly)
+        readCompletionNotifier->setEnabled(true);
+
+    if (mode & QIODevice::WriteOnly)
+        writeCompletionNotifier->setEnabled(true);
+
+    if (!::SetCommMask(handle, originalEventMask)) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    if (!startAsyncCommunication())
+        return false;
+
+    communicationNotifier->setEnabled(true);
+
+    return true;
+}
+
 bool QSerialPortPrivate::updateDcb()
 {
     Q_Q(QSerialPort);
diff --git a/src/serialport/qserialport_win_p.h b/src/serialport/qserialport_win_p.h
index daf5788..a997315 100644
--- a/src/serialport/qserialport_win_p.h
+++ b/src/serialport/qserialport_win_p.h
@@ -130,6 +130,7 @@ public:
     DWORD triggeredEventMask;
 
 private:
+    bool initialize(QIODevice::OpenMode mode);
     bool updateDcb();
     bool updateCommTimeouts();
     qint64 handleOverlappedResult(int direction, OVERLAPPED &overlapped);
diff --git a/src/serialport/qserialport_wince.cpp b/src/serialport/qserialport_wince.cpp
index 4a523ae..c06e748 100644
--- a/src/serialport/qserialport_wince.cpp
+++ b/src/serialport/qserialport_wince.cpp
@@ -219,43 +219,11 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
         return false;
     }
 
-    ::ZeroMemory(&restoredDcb, sizeof(restoredDcb));
-    restoredDcb.DCBlength = sizeof(restoredDcb);
-
-    if (!::GetCommState(handle, &restoredDcb)) {
-        q->setError(decodeSystemError());
-        return false;
-    }
-
-    currentDcb = restoredDcb;
-    currentDcb.fBinary = true;
-    currentDcb.fInX = false;
-    currentDcb.fOutX = false;
-    currentDcb.fAbortOnError = false;
-    currentDcb.fNull = false;
-    currentDcb.fErrorChar = false;
-
-    if (currentDcb.fDtrControl ==  DTR_CONTROL_HANDSHAKE)
-        currentDcb.fDtrControl = DTR_CONTROL_DISABLE;
-
-    if (!updateDcb())
-        return false;
-
-    if (!::GetCommTimeouts(handle, &restoredCommTimeouts)) {
-        q->setError(decodeSystemError());
-        return false;
-    }
-
-    ::memset(&currentCommTimeouts, 0, sizeof(currentCommTimeouts));
-    currentCommTimeouts.ReadIntervalTimeout = MAXDWORD;
-
-    if (!updateCommTimeouts())
-        return false;
-
-    eventNotifier = new CommEventNotifier(eventMask, this, q);
-    eventNotifier->start();
+    if (initialize(eventMask))
+        return true;
 
-    return true;
+    ::CloseHandle(handle);
+    return false;
 }
 
 void QSerialPortPrivate::close()
@@ -646,6 +614,49 @@ void QSerialPortPrivate::processIoErrors(bool error)
     }
 }
 
+inline bool QSerialPortPrivate::initialize(DWORD eventMask)
+{
+    Q_Q(QSerialPort);
+
+    ::ZeroMemory(&restoredDcb, sizeof(restoredDcb));
+    restoredDcb.DCBlength = sizeof(restoredDcb);
+
+    if (!::GetCommState(handle, &restoredDcb)) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    currentDcb = restoredDcb;
+    currentDcb.fBinary = true;
+    currentDcb.fInX = false;
+    currentDcb.fOutX = false;
+    currentDcb.fAbortOnError = false;
+    currentDcb.fNull = false;
+    currentDcb.fErrorChar = false;
+
+    if (currentDcb.fDtrControl ==  DTR_CONTROL_HANDSHAKE)
+        currentDcb.fDtrControl = DTR_CONTROL_DISABLE;
+
+    if (!updateDcb())
+        return false;
+
+    if (!::GetCommTimeouts(handle, &restoredCommTimeouts)) {
+        q->setError(decodeSystemError());
+        return false;
+    }
+
+    ::memset(&currentCommTimeouts, 0, sizeof(currentCommTimeouts));
+    currentCommTimeouts.ReadIntervalTimeout = MAXDWORD;
+
+    if (!updateCommTimeouts())
+        return false;
+
+    eventNotifier = new CommEventNotifier(eventMask, this, q);
+    eventNotifier->start();
+
+    return true;
+}
+
 bool QSerialPortPrivate::updateDcb()
 {
     Q_Q(QSerialPort);
diff --git a/src/serialport/qserialport_wince_p.h b/src/serialport/qserialport_wince_p.h
index 11d0534..9697dc7 100644
--- a/src/serialport/qserialport_wince_p.h
+++ b/src/serialport/qserialport_wince_p.h
@@ -112,6 +112,7 @@ public:
     QMutex settingsChangeMutex;
 
 private:
+    bool initialize(DWORD eventMask);
     bool updateDcb();
     bool updateCommTimeouts();