QtCore: don't use QString::utf16() where the string is \0-terminated

As requested in the code review, use str.data_ptr().data() to skip the detaching check where we need a non-const pointer and it's immediately clear the string isn't shared. Amends 525aff1. Task-number: QTBUG-125871 Change-Id: Ib4df35fcbeac8778fbc2c5acfabdef52f9360df3 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
qt · Nov 13, 2024 · 29a6cb3 · 29a6cb3
1 parent 00cb46b
commit 29a6cb3
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 20 deletions.
diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt
@@ -148,6 +148,7 @@ qt_internal_add_module(Core
         io/qurlquery.cpp io/qurlquery.h
         io/qurlrecode.cpp
         io/qzipreader_p.h io/qzipwriter_p.h io/qzip.cpp
+        io/wcharhelpers_win_p.h
         kernel/qabstracteventdispatcher.cpp kernel/qabstracteventdispatcher.h kernel/qabstracteventdispatcher_p.h
         kernel/qabstractnativeeventfilter.cpp kernel/qabstractnativeeventfilter.h
         kernel/qapplicationstatic.h

diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp
@@ -40,6 +40,7 @@
 #include <cstdio>
 
 #include <QtCore/private/qfunctions_win_p.h>
+#include <QtCore/private/wcharhelpers_win_p.h>
 
 #ifndef SPI_GETPLATFORMTYPE
 #define SPI_GETPLATFORMTYPE 257
@@ -684,8 +685,7 @@ static QString readSymLink(const QFileSystemEntry &link)
             DWORD len;
             wchar_t buffer[MAX_PATH];
             const QString volumeName = "\\\\?\\"_L1 + matchVolume.captured();
-            if (GetVolumePathNamesForVolumeName(reinterpret_cast<LPCWSTR>(volumeName.unicode()),
-                                                buffer, MAX_PATH, &len)
+            if (GetVolumePathNamesForVolumeName(qt_castToWchar(volumeName), buffer, MAX_PATH, &len)
                 != 0) {
                 result.replace(0, matchVolume.capturedLength(), QString::fromWCharArray(buffer));
             }

diff --git a/src/corelib/io/qfilesystemiterator_win.cpp b/src/corelib/io/qfilesystemiterator_win.cpp
@@ -7,6 +7,7 @@
 #include "qplatformdefs.h"
 
 #include <QtCore/qt_windows.h>
+#include <QtCore/private/wcharhelpers_win_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -69,7 +70,8 @@ bool QFileSystemIterator::advance(QFileSystemEntry &fileEntry, QFileSystemMetaDa
         int searchOps =  0;         // FindExSearchNameMatch
         if (onlyDirs)
             searchOps = 1 ;         // FindExSearchLimitToDirectories
-        findFileHandle = FindFirstFileEx((const wchar_t *)nativePath.utf16(), FINDEX_INFO_LEVELS(infoLevel), &findData,
+        findFileHandle = FindFirstFileEx(qt_castToWchar(nativePath),
+                                         FINDEX_INFO_LEVELS(infoLevel), &findData,
                                          FINDEX_SEARCH_OPS(searchOps), 0, dwAdditionalFlags);
         if (findFileHandle == INVALID_HANDLE_VALUE) {
             if (nativePath.startsWith("\\\\?\\UNC\\"_L1)) {

diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
@@ -544,7 +544,7 @@ void QProcessPrivate::startProcess()
         return;
     }
 
-    const QString args = qt_create_commandline(program, arguments, nativeArguments);
+    QString args = qt_create_commandline(program, arguments, nativeArguments);
     QByteArray envlist;
     if (!environment.inheritsFromParent())
         envlist = qt_create_environment(environment.d.constData()->vars);
@@ -566,7 +566,7 @@ void QProcessPrivate::startProcess()
     STARTUPINFOW startupInfo = createStartupInfo();
     const QString nativeWorkingDirectory = QDir::toNativeSeparators(workingDirectory);
     QProcess::CreateProcessArguments cpargs = {
-        nullptr, reinterpret_cast<wchar_t *>(const_cast<ushort *>(args.utf16())),
+        nullptr, reinterpret_cast<wchar_t *>(args.data_ptr().data()),
         nullptr, nullptr, true, dwCreationFlags,
         environment.inheritsFromParent() ? nullptr : envlist.data(),
         nativeWorkingDirectory.isEmpty()
@@ -920,7 +920,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
     dwCreationFlags |= CREATE_UNICODE_ENVIRONMENT;
     STARTUPINFOW startupInfo = createStartupInfo();
     QProcess::CreateProcessArguments cpargs = {
-        nullptr, reinterpret_cast<wchar_t *>(const_cast<ushort *>(args.utf16())),
+        nullptr, reinterpret_cast<wchar_t *>(args.data_ptr().data()),
         nullptr, nullptr, true, dwCreationFlags, envPtr,
         workingDirectory.isEmpty()
             ? nullptr : reinterpret_cast<const wchar_t *>(workingDirectory.utf16()),

diff --git a/src/corelib/io/qsettings_win.cpp b/src/corelib/io/qsettings_win.cpp
@@ -8,6 +8,8 @@
 #include "qmap.h"
 #include "qdebug.h"
 #include "qscopeguard.h"
+#include <QtCore/private/wcharhelpers_win_p.h>
+
 #include <qt_windows.h>
 
 // See "Accessing an Alternate Registry View" at:
@@ -262,7 +264,7 @@ static void deleteChildGroups(HKEY parentHandle, REGSAM access = 0)
         RegCloseKey(childGroupHandle);
 
         // delete group itself
-        LONG res = RegDeleteKey(parentHandle, reinterpret_cast<const wchar_t *>(group.utf16()));
+        LONG res = RegDeleteKey(parentHandle, qt_castToWchar(group));
         if (res != ERROR_SUCCESS) {
             qErrnoWarning(int(res), "QSettings: RegDeleteKey failed on subkey \"%ls\"",
                           qUtf16Printable(group));
@@ -584,7 +586,7 @@ void QWinSettingsPrivate::remove(const QString &uKey)
             const QStringList childKeys = childKeysOrGroups(handle, QSettingsPrivate::ChildKeys);
 
             for (const QString &group : childKeys) {
-                LONG res = RegDeleteValue(handle, reinterpret_cast<const wchar_t *>(group.utf16()));
+                LONG res = RegDeleteValue(handle, qt_castToWchar(group));
                 if (res != ERROR_SUCCESS) {
                     qErrnoWarning(int(res), "QSettings: RegDeleteValue failed on subkey \"%ls\"",
                                   qUtf16Printable(group));
@@ -678,7 +680,7 @@ void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
             int length = s.length();
             if (type == REG_SZ)
                 ++length;
-            regValueBuff = QByteArray(reinterpret_cast<const char *>(s.utf16()),
+            regValueBuff = QByteArray(reinterpret_cast<const char *>(s.constData()),
                                       int(sizeof(wchar_t)) * length);
             break;
         }

diff --git a/src/corelib/io/qstorageinfo_win.cpp b/src/corelib/io/qstorageinfo_win.cpp
@@ -7,6 +7,7 @@
 #include <QtCore/qfileinfo.h>
 #include <QtCore/qmutex.h>
 #include <QtCore/qvarlengtharray.h>
+#include <QtCore/private/wcharhelpers_win_p.h>
 
 #include "qfilesystementry_p.h"
 #include "private/qsystemlibrary_p.h"
@@ -176,7 +177,7 @@ bool QStorageInfoPrivate::queryStorageProperty()
     if (path.endsWith(u'\\'))
         path.chop(1);
 
-    HANDLE handle = CreateFile(reinterpret_cast<const wchar_t *>(path.utf16()),
+    HANDLE handle = CreateFile(qt_castToWchar(path),
                                0, // no access to the drive
                                FILE_SHARE_READ | FILE_SHARE_WRITE,
                                nullptr,
@@ -276,7 +277,7 @@ void QStorageInfoPrivate::queryFileFsSectorSizeInformation()
         path.append(u'\\');
 
     UNICODE_STRING name;
-    qtRtlInitUnicodeString(&name, reinterpret_cast<const wchar_t *>(path.utf16()));
+    qtRtlInitUnicodeString(&name, qt_castToWchar(path));
 
     InitializeObjectAttributes(&attrs, &name, 0, nullptr, nullptr);
 

diff --git a/src/corelib/io/wcharhelpers_win_p.h b/src/corelib/io/wcharhelpers_win_p.h
@@ -0,0 +1,30 @@
+// Copyright (C) 2024 Ahmad Samir <a.samirh78@gmail.com>
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef WCHARHELPERS_WIN_P_H
+#define WCHARHELPERS_WIN_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists purely as an
+// implementation detail.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+
+#include <QtCore/qstring.h>
+
+QT_BEGIN_NAMESPACE
+
+static inline const wchar_t *qt_castToWchar(const QString &s)
+{
+    return reinterpret_cast<const wchar_t *>(s.constData());
+}
+
+QT_END_NAMESPACE
+
+#endif // WCHARHELPERS_WIN_P_H
diff --git a/src/corelib/plugin/qsystemlibrary.cpp b/src/corelib/plugin/qsystemlibrary.cpp
@@ -5,6 +5,7 @@
 #include <QtCore/qvarlengtharray.h>
 #include <QtCore/qstringlist.h>
 #include <QtCore/qfileinfo.h>
+#include <QtCore/private/wcharhelpers_win_p.h>
 
 /*!
 
@@ -81,8 +82,7 @@ HINSTANCE QSystemLibrary::load(const wchar_t *libraryName, bool onlySystemDirect
             fullPathAttempt.append(u'\\');
         }
         fullPathAttempt.append(fileName);
-        HINSTANCE inst =
-            ::LoadLibrary(reinterpret_cast<const wchar_t *>(fullPathAttempt.unicode()));
+        HINSTANCE inst = ::LoadLibrary(qt_castToWchar(fullPathAttempt));
         if (inst != nullptr)
             return inst;
     }

diff --git a/src/corelib/text/qstringconverter.cpp b/src/corelib/text/qstringconverter.cpp
@@ -24,6 +24,8 @@
 #include <qt_windows.h>
 #ifndef QT_BOOTSTRAPPED
 #include <QtCore/qvarlengtharray.h>
+#include <QtCore/private/wcharhelpers_win_p.h>
+
 #include <QtCore/q20iterator.h>
 #include <QtCore/q26numeric.h>
 #endif // !QT_BOOTSTRAPPED
@@ -1797,9 +1799,8 @@ QByteArray QLocal8Bit::convertFromUnicode_sys(QStringView in, quint32 codePage,
 #ifndef QT_NO_DEBUG
                 // Can't use qWarning(), as it'll recurse to handle %ls
                 fprintf(stderr,
-                        "WideCharToMultiByte: Cannot convert multibyte text (error %d): %ls\n", r,
-                        reinterpret_cast<const wchar_t *>(
-                                QStringView(ch, uclen).left(100).toString().unicode()));
+                        "WideCharToMultiByte: Cannot convert multibyte text (error %d): %ls\n",
+                        r, qt_castToWchar(QStringView(ch, uclen).left(100).toString()));
 #endif
                 break;
             }

diff --git a/src/corelib/time/qtimezoneprivate_win.cpp b/src/corelib/time/qtimezoneprivate_win.cpp
@@ -8,6 +8,7 @@
 #include "qdatetime.h"
 #include "qdebug.h"
 #include <private/qnumeric_p.h>
+#include <private/wcharhelpers_win_p.h>
 
 #include <algorithm>
 
@@ -537,10 +538,9 @@ void QWinTimeZonePrivate::init(const QByteArray &ianaId)
                 const int endYear = dynamicKey.value<int>(L"LastEntry").value_or(0);
                 for (int year = startYear; year <= endYear; ++year) {
                     bool ruleOk;
-                    QWinTransitionRule rule =
-                        readRegistryRule(dynamicKey,
-                                         reinterpret_cast<LPCWSTR>(QString::number(year).unicode()),
-                                         &ruleOk);
+                    QWinTransitionRule rule = readRegistryRule(dynamicKey,
+                                                               qt_castToWchar(QString::number(year)),
+                                                               &ruleOk);
                     if (ruleOk
                         // Don't repeat a recurrent rule:
                         && (m_tranRules.isEmpty()