Replace bStrFromQString with a QBStr RAII wrapper

This makes transfer of ownership explicit. The code is from ActiveQt.
If this patch is accepted, it can be removed from ActiveQt.

Task-number: QTBUG-126530
Change-Id: I613004ba784f87a9b935b2bbaead2205243c3033
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
(cherry picked from commit 93686386c078e2be03fb8bc42dee60a9e36fc23f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/corelib/CMakeLists.txt

@@ -562,6 +562,7 @@ qt_internal_extend_target(Core CONDITION WIN32
         thread/qthread_win.cpp
         platform/windows/qcomobject_p.h
         platform/windows/qcomptr_p.h
+        platform/windows/qbstr_p.h
     LIBRARIES
         advapi32
         authz

src/corelib/platform/windows/qbstr_p.h

@@ -0,0 +1,144 @@
+// Copyright (C) 2024 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
+
+#ifndef QBSTR_P_H
+#define QBSTR_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/private/qglobal_p.h>
+
+#if defined(Q_OS_WIN) || defined(Q_QDOC)
+
+#include <QtCore/qt_windows.h>
+#include <QtCore/qstring.h>
+#include <utility>
+#include <oaidl.h>
+
+QT_BEGIN_NAMESPACE
+
+class QBStr
+{
+public:
+    QBStr() = default;
+
+    ~QBStr() noexcept
+    {
+        free();
+    }
+
+    // Does not take ownership
+    explicit QBStr(LPCOLESTR str) noexcept
+    {
+        if (str)
+            m_str = ::SysAllocString(str);
+        Q_ASSERT(m_str || !str);
+    }
+
+    explicit QBStr(const QString &str) noexcept
+    {
+        if (!str.isNull())
+            m_str = ::SysAllocString(reinterpret_cast<const wchar_t *>(str.utf16()));
+        Q_ASSERT(m_str || str.isNull());
+    }
+
+    QBStr(const QBStr &str) noexcept : m_str{ str.copy() } { }
+
+    QBStr(QBStr &&str) noexcept : m_str{ std::exchange(str.m_str, nullptr) } { }
+
+    // Does not take ownership
+    QBStr &operator=(LPCOLESTR str) noexcept
+    {
+        free();
+        if (str)
+            m_str = ::SysAllocString(str);
+        Q_ASSERT(m_str || !str);
+        return *this;
+    }
+
+    QBStr &operator=(const QString &str) noexcept
+    {
+        free();
+        if (!str.isNull())
+            m_str = ::SysAllocString(reinterpret_cast<const wchar_t*>(str.utf16()));
+        Q_ASSERT(m_str || str.isNull());
+        return *this;
+    }
+
+    QBStr &operator=(const QBStr &rhs) noexcept
+    {
+        if (this != std::addressof(rhs))
+            reset(rhs.copy());
+
+        return *this;
+    }
+
+    QBStr &operator=(QBStr &&rhs) noexcept
+    {
+        if (this != std::addressof(rhs))
+            reset(std::exchange(rhs.m_str, nullptr));
+
+        return *this;
+    }
+
+    const BSTR &bstr() const noexcept
+    {
+        return m_str;
+    }
+
+    QString str() const
+    {
+        return QString::fromWCharArray(m_str);
+    }
+
+    [[nodiscard]] BSTR release() noexcept
+    {
+        return std::exchange(m_str, nullptr);
+    }
+
+    [[nodiscard]] BSTR *operator&() noexcept // NOLINT(google-runtime-operator)
+    {
+        Q_ASSERT(!m_str);
+        return &m_str;
+    }
+
+private:
+    void free() noexcept
+    {
+        if (m_str)
+            ::SysFreeString(m_str);
+        m_str = nullptr;
+    }
+
+    void reset(BSTR str) noexcept
+    {
+        free();
+        m_str = str;
+    }
+
+    [[nodiscard]] BSTR copy() const noexcept
+    {
+        if (!m_str)
+            return nullptr;
+
+        return ::SysAllocStringByteLen(reinterpret_cast<const char *>(m_str),
+                                       ::SysStringByteLen(m_str));
+    }
+
+    BSTR m_str = nullptr;
+};
+
+QT_END_NAMESPACE
+
+#endif // Q_OS_WIN
+
+#endif // QCOMPTR_P_H

src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp

@@ -213,17 +213,15 @@ void QWindowsUiaMainProvider::raiseNotification(QAccessibleAnnouncementEvent *ev
 {
     if (QAccessibleInterface *accessible = event->accessibleInterface()) {
         if (auto provider = providerForAccessible(accessible)) {
-            BSTR message = bStrFromQString(event->message());
+            QBStr message{ event->message() };
             QAccessible::AnnouncementPoliteness prio = event->politeness();
             NotificationProcessing processing = (prio == QAccessible::AnnouncementPoliteness::Assertive)
                     ? NotificationProcessing_ImportantAll
                     : NotificationProcessing_All;
-            BSTR activityId = bStrFromQString(QString::fromLatin1(""));
-            UiaRaiseNotificationEvent(provider.Get(), NotificationKind_Other, processing, message,
-                                      activityId);
+            QBStr activityId{ QString::fromLatin1("") };
+            UiaRaiseNotificationEvent(provider.Get(), NotificationKind_Other, processing, message.bstr(),
+                                      activityId.bstr());
 
-            ::SysFreeString(message);
-            ::SysFreeString(activityId);
         }
     }
 }

src/plugins/platforms/windows/uiautomation/qwindowsuiatextrangeprovider.cpp

@@ -293,7 +293,7 @@ HRESULT QWindowsUiaTextRangeProvider::GetText(int maxLength, BSTR *pRetVal)
 
     if ((maxLength > -1) && (rangeText.size() > maxLength))
         rangeText.truncate(maxLength);
-    *pRetVal = bStrFromQString(rangeText);
+    *pRetVal = QBStr(rangeText).release();
     return S_OK;
 }
 

src/plugins/platforms/windows/uiautomation/qwindowsuiautils.cpp

@@ -81,15 +81,10 @@ void setVariantDouble(double value, VARIANT *variant)
     variant->dblVal = value;
 }
 
-BSTR bStrFromQString(const QString &value)
-{
-    return SysAllocString(reinterpret_cast<const wchar_t *>(value.utf16()));
-}
-
 void setVariantString(const QString &value, VARIANT *variant)
 {
     variant->vt = VT_BSTR;
-    variant->bstrVal = bStrFromQString(value);
+    variant->bstrVal = QBStr(value).release();
 }
 
 // Scales a rect to native coordinates, according to high dpi settings.

src/plugins/platforms/windows/uiautomation/qwindowsuiautils.h

@@ -12,6 +12,7 @@
 #include <QtGui/qaccessible.h>
 #include <QtGui/qwindow.h>
 #include <QtCore/qrect.h>
+#include <QtCore/private/qbstr_p.h>
 #include "qwindowsuiautomation.h"
 
 QT_BEGIN_NAMESPACE
@@ -38,8 +39,6 @@ void setVariantBool(bool value, VARIANT *variant);
 
 void setVariantDouble(double value, VARIANT *variant);
 
-BSTR bStrFromQString(const QString &value);
-
 void setVariantString(const QString &value, VARIANT *variant);
 
 } // namespace QWindowsUiAutomation

src/plugins/platforms/windows/uiautomation/qwindowsuiavalueprovider.cpp

@@ -86,7 +86,7 @@ HRESULT STDMETHODCALLTYPE QWindowsUiaValueProvider::get_Value(BSTR *pRetVal)
     if (!accessible)
         return UIA_E_ELEMENTNOTAVAILABLE;
 
-    *pRetVal = bStrFromQString(accessible->text(QAccessible::Value));
+    *pRetVal = QBStr(accessible->text(QAccessible::Value)).release();
     return S_OK;
 }
 

tests/auto/corelib/platform/windows/CMakeLists.txt

@@ -2,3 +2,4 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 add_subdirectory(qcomobject)
+add_subdirectory(qbstr)

tests/auto/corelib/platform/windows/qbstr/CMakeLists.txt

@@ -0,0 +1,15 @@
+# Copyright (C) 2024 The Qt Company Ltd.
+# SPDX-License-Identifier: BSD-3-Clause
+
+if(NOT QT_BUILD_STANDALONE_TESTS AND NOT QT_BUILDING_QT)
+    cmake_minimum_required(VERSION 3.16)
+    project(tst_qbstr LANGUAGES CXX)
+    find_package(Qt6BuildInternals REQUIRED COMPONENTS STANDALONE_TEST)
+endif()
+
+qt_internal_add_test(tst_qbstr
+    SOURCES
+        tst_qbstr.cpp
+    LIBRARIES
+        Qt::CorePrivate
+)

tests/auto/corelib/platform/windows/qbstr/tst_qbstr.cpp

@@ -0,0 +1,227 @@
+// Copyright (C) 2016 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include <QtTest/QtTest>
+
+#ifdef Q_OS_WIN
+
+#include <QtCore/private/qbstr_p.h>
+
+QT_USE_NAMESPACE
+
+using namespace Qt::StringLiterals;
+
+typedef int (*SETOANOCACHE)(void);
+
+void DisableBSTRCache()
+{
+    const HINSTANCE hLib = LoadLibraryA("OLEAUT32.DLL");
+    if (hLib != nullptr) {
+        const SETOANOCACHE SetOaNoCache =
+                reinterpret_cast<SETOANOCACHE>(GetProcAddress(hLib, "SetOaNoCache"));
+        if (SetOaNoCache != nullptr)
+            SetOaNoCache();
+        FreeLibrary(hLib);
+    }
+}
+
+class tst_qbstr : public QObject
+{
+    Q_OBJECT
+private slots:
+    void initTestCase()
+    {
+        DisableBSTRCache();
+    }
+
+    void constructor_nullInitializes()
+    {
+        const QBStr str;
+        QVERIFY(!str.bstr());
+    }
+
+    void constructor_initializesToNullptr_withNullptr()
+    {
+        const QBStr str{ nullptr };
+        QCOMPARE_EQ(str.bstr(), nullptr);
+    }
+
+    void constructor_initializes_withLiteral()
+    {
+        const QBStr str{ L"hello world" };
+        QCOMPARE_EQ(str.bstr(), "hello world"_L1);
+    }
+
+    void constructor_initializes_withQString()
+    {
+        const QString expected{ "hello world"_L1 };
+        QBStr str{ expected };
+        QCOMPARE_EQ(QStringView{ str.bstr() }, expected);
+    }
+
+    void constructor_initializesToNullptr_withNullQString()
+    {
+        const QString expected{ };
+        QBStr str{ expected };
+        QCOMPARE_EQ(str.bstr(), nullptr);
+    }
+
+    void constructor_initializes_withEmptyQString()
+    {
+        const QString expected{ ""_L1 };
+        QBStr str{ expected };
+        QCOMPARE_EQ(QStringView{ str.bstr() }, expected);
+        QCOMPARE_NE(str.bstr(), nullptr);
+    }
+
+    void copyConstructor_initializesToNullptr_withNullString()
+    {
+        const QBStr null;
+        const QBStr dest = null;
+        QCOMPARE_EQ(dest.bstr(), nullptr);
+    }
+
+    void copyConstructor_initializes_withValidQBstr()
+    {
+        const QBStr expected{ L"hello world" };
+        const QBStr dest = expected;
+        QCOMPARE_EQ(QStringView{ dest.bstr() }, expected.bstr());
+    }
+
+    void moveConstructor_initializesToNullptr_withNullQBstr()
+    {
+        const QBStr str{ QBStr{} };
+        QCOMPARE_EQ(str.bstr(), nullptr);
+    }
+
+    void moveConstructor_initializes_withValidQBstr()
+    {
+        QBStr source { L"hello world" };
+        const QBStr str{ std::move(source) };
+        QCOMPARE_EQ(str.bstr(), "hello world"_L1);
+    }
+
+    void assignment_assigns_withNullptr()
+    {
+        QBStr value{};
+        value = nullptr;
+        QCOMPARE_EQ(value.bstr(), nullptr);
+    }
+
+    void assignment_assigns_WCharPointer()
+    {
+        QBStr value{};
+        const wchar_t utf16[] = L"Hello world";
+        const wchar_t *ptr = utf16;
+        value = ptr;
+        QCOMPARE_EQ(value.bstr(), "Hello world"_L1);
+    }
+
+    void assignment_assigns_WCharLiteral()
+    {
+        QBStr value{};
+        value = L"Hello world";
+        QCOMPARE_EQ(value.bstr(), "Hello world"_L1);
+    }
+
+    void assignment_assignsEmpty_EmptyWCharLiteral()
+    {
+        QBStr value{};
+        value = L"";
+        QCOMPARE_EQ(value.bstr(), ""_L1);
+        QCOMPARE_NE(value.bstr(), nullptr);
+    }
+
+    void assignment_assigns_withQString()
+    {
+        QBStr value{};
+        value = QString{ "Hello world"_L1 };
+        QCOMPARE_EQ(value.bstr(), "Hello world"_L1);
+    }
+
+    void assignment_assignsNullptr_withNullQString()
+    {
+        QBStr value{};
+        value = QString{};
+        QCOMPARE_EQ(value.bstr(), nullptr);
+    }
+
+    void assignment_assignsEmpty_withEmptyQString()
+    {
+        QBStr value{};
+        value = QString{""_L1};
+        QCOMPARE_EQ(value.bstr(), ""_L1);
+        QCOMPARE_NE(value.bstr(), nullptr);
+    }
+
+    void assignment_assignsNullptr_withNullQBStr()
+    {
+        const QBStr source{};
+        const QBStr dest = source;
+        QCOMPARE_EQ(dest.bstr(), nullptr);
+    }
+
+    void assignment_assigns_withValidQBStr()
+    {
+        const QBStr source{ L"hello world" };
+        const QBStr dest = source;
+        QCOMPARE_EQ(dest.bstr(), "hello world"_L1);
+    }
+
+    void assignment_assigns_withSelfAssignment()
+    {
+        QBStr value{ L"hello world" };
+        value = value;
+        QCOMPARE_EQ(value.bstr(), "hello world"_L1);
+    }
+
+    void moveAssignment_assigns_withNullQBStr()
+    {
+        QBStr source{};
+        const QBStr dest = std::move(source);
+        QCOMPARE_EQ(dest.bstr(), nullptr);
+    }
+
+    void moveAssignment_assigns_withValidQBStr()
+    {
+        QBStr source{ L"hello world" };
+        const QBStr dest = std::move(source);
+        QCOMPARE_EQ(dest.bstr(), "hello world"_L1);
+    }
+
+    void moveAssignment_assigns_withSelfAssignment()
+    {
+        QBStr value{ L"hello world" };
+        value = std::move(value);
+        QCOMPARE_EQ(value.bstr(), "hello world"_L1);
+    }
+
+    void release_returnsStringAndClearsValue()
+    {
+        QBStr str{ L"hello world" };
+        BSTR val = str.release();
+        QCOMPARE_EQ(str.bstr(), nullptr);
+        QCOMPARE_EQ(val, "hello world"_L1);
+        SysFreeString(val);
+    }
+
+    void str_returnsNullQString_withNullQBStr()
+    {
+        const QBStr bstr{};
+        const QString str = bstr.str();
+        QVERIFY(str.isNull());
+    }
+
+    void str_returnsQString_withValidQBStr()
+    {
+        const QBStr bstr{L"hello world"};
+        const QString str = bstr.str();
+        QCOMPARE_EQ(str, "hello world");
+    }
+
+};
+
+QTEST_MAIN(tst_qbstr)
+#include "tst_qbstr.moc"
+
+#endif // Q_OS_WIN