From 96b9590dbeb20e000d9d9308bee2162b1101a7e0 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Fri, 2 Oct 2015 17:00:47 +0300 Subject: Refactor QSPI code on Windows Current approach with passing of the array's data pointer to a Win API functions when the array is empty has potential danger. Because the data pointer is invalid, and if a Win API function tries to get access to this pointer then this can cause a crash or an UB. Still this does not happens, but nobody guarantees that this will be same in future. It is better to pre-allocate the array with some size (e.g. with the MAX_PATH size). In this case we can pass the data pointer without any problem. Besides, the Win API functions will return result with the first pass if the array size is sufficient; otherwise it returns ERROR_MORE_DATA error code and we can re-allocate the array with required size and to return a result in the second pass. We re-allocate the array with size more than required on some bytes with filling of the array with zeros. This allows us to convert it using QString::fromWCharArray() without specifying a size, and to remove the toStringAndTrimNullCharacter() function. Change-Id: I5976485286db4097514ea7b3a8adfc9a6a7cea4f Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_win.cpp | 74 ++++++++++++++++------------------ 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/src/serialport/qserialportinfo_win.cpp b/src/serialport/qserialportinfo_win.cpp index 38190b7..06987f2 100644 --- a/src/serialport/qserialportinfo_win.cpp +++ b/src/serialport/qserialportinfo_win.cpp @@ -41,6 +41,8 @@ #include #include +#include + #include #include #include @@ -63,14 +65,6 @@ static inline const QList& guidFlagsPairs() return guidFlagsPairList; } -static QString toStringAndTrimNullCharacter(const QByteArray &buffer) -{ - const QString result = QString::fromWCharArray(reinterpret_cast(buffer.constData()), - buffer.size() / sizeof(wchar_t)); - const int index = result.indexOf(QChar(0)); - return index == -1 ? result : result.mid(0, index); -} - static QStringList portNamesFromHardwareDeviceMap() { HKEY hKey = Q_NULLPTR; @@ -79,19 +73,22 @@ static QStringList portNamesFromHardwareDeviceMap() QStringList result; DWORD index = 0; - static const DWORD maximumValueNameInChars = 16383; - QByteArray outputValueName; - outputValueName.resize(maximumValueNameInChars * sizeof(wchar_t)); - QByteArray outputBuffer; - DWORD requiredDataBytes = 0; + + // This is a maximum length of value name, see: + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724872%28v=vs.85%29.aspx + enum { MaximumValueNameInChars = 16383 }; + + std::vector outputValueName(MaximumValueNameInChars, 0); + std::vector outputBuffer(MAX_PATH + 1, 0); + DWORD bytesRequired = MAX_PATH; forever { - DWORD requiredValueNameChars = maximumValueNameInChars; - const LONG ret = ::RegEnumValue(hKey, index, reinterpret_cast(outputValueName.data()), &requiredValueNameChars, - Q_NULLPTR, Q_NULLPTR, reinterpret_cast(outputBuffer.data()), &requiredDataBytes); + DWORD requiredValueNameChars = MaximumValueNameInChars; + const LONG ret = ::RegEnumValue(hKey, index, &outputValueName[0], &requiredValueNameChars, + Q_NULLPTR, Q_NULLPTR, reinterpret_cast(&outputBuffer[0]), &bytesRequired); if (ret == ERROR_MORE_DATA) { - outputBuffer.resize(requiredDataBytes); + outputBuffer.resize(bytesRequired / sizeof(wchar_t) + 2, 0); } else if (ret == ERROR_SUCCESS) { - result.append(toStringAndTrimNullCharacter(outputBuffer)); + result.append(QString::fromWCharArray(&outputBuffer[0])); ++index; } else { break; @@ -106,12 +103,12 @@ static QString deviceRegistryProperty(HDEVINFO deviceInfoSet, DWORD property) { DWORD dataType = 0; - QByteArray devicePropertyByteArray; - DWORD requiredSize = 0; + std::vector outputBuffer(MAX_PATH + 1, 0); + DWORD bytesRequired = MAX_PATH; forever { if (::SetupDiGetDeviceRegistryProperty(deviceInfoSet, deviceInfoData, property, &dataType, - reinterpret_cast(devicePropertyByteArray.data()), - devicePropertyByteArray.size(), &requiredSize)) { + reinterpret_cast(&outputBuffer[0]), + bytesRequired, &bytesRequired)) { break; } @@ -119,25 +116,22 @@ static QString deviceRegistryProperty(HDEVINFO deviceInfoSet, || (dataType != REG_SZ && dataType != REG_EXPAND_SZ)) { return QString(); } - devicePropertyByteArray.resize(requiredSize); + outputBuffer.resize(bytesRequired / sizeof(wchar_t) + 2, 0); } - return toStringAndTrimNullCharacter(devicePropertyByteArray); + return QString::fromWCharArray(&outputBuffer[0]); } static QString deviceInstanceIdentifier(DEVINST deviceInstanceNumber) { - ULONG numberOfChars = 0; - if (::CM_Get_Device_ID_Size(&numberOfChars, deviceInstanceNumber, 0) != CR_SUCCESS) - return QString(); - // The size does not include the terminating null character. - ++numberOfChars; - QByteArray outputBuffer; - outputBuffer.resize(numberOfChars * sizeof(wchar_t)); - if (::CM_Get_Device_ID(deviceInstanceNumber, reinterpret_cast(outputBuffer.data()), - outputBuffer.size(), 0) != CR_SUCCESS) { + std::vector outputBuffer(MAX_DEVICE_ID_LEN + 1, 0); + if (::CM_Get_Device_ID( + deviceInstanceNumber, + &outputBuffer[0], + MAX_DEVICE_ID_LEN, + 0) != CR_SUCCESS) { return QString(); } - return toStringAndTrimNullCharacter(outputBuffer).toUpper(); + return QString::fromWCharArray(&outputBuffer[0]).toUpper(); } static DEVINST parentDeviceInstanceNumber(DEVINST childDeviceInstanceNumber) @@ -167,20 +161,20 @@ static QString devicePortName(HDEVINFO deviceInfoSet, PSP_DEVINFO_DATA deviceInf QString portName; foreach (const QString &portNameKey, portNameRegistryKeyList) { - DWORD bytesRequired = 0; DWORD dataType = 0; - QByteArray outputBuffer; + std::vector outputBuffer(MAX_PATH + 1, 0); + DWORD bytesRequired = MAX_PATH; forever { const LONG ret = ::RegQueryValueEx(key, reinterpret_cast(portNameKey.utf16()), Q_NULLPTR, &dataType, - reinterpret_cast(outputBuffer.data()), &bytesRequired); + reinterpret_cast(&outputBuffer[0]), &bytesRequired); if (ret == ERROR_MORE_DATA) { - outputBuffer.resize(bytesRequired); + outputBuffer.resize(bytesRequired / sizeof(wchar_t) + 2, 0); continue; } else if (ret == ERROR_SUCCESS) { if (dataType == REG_SZ) - portName = toStringAndTrimNullCharacter(outputBuffer); + portName = QString::fromWCharArray(&outputBuffer[0]); else if (dataType == REG_DWORD) - portName = QStringLiteral("COM%1").arg(*(PDWORD(outputBuffer.constData()))); + portName = QStringLiteral("COM%1").arg(*(PDWORD(&outputBuffer[0]))); } break; } -- cgit v1.2.1