Introduce QBStr BSTR RAII wrapper

Use of base BSTR instances can be error prone and verbose. This patch
introduces a QBStr class that wraps the Microsoft BSTR string type.

Code that can be tested is updated to use the new QBStr instead of bare
BSTR instances.

Change-Id: If55e7d18426d1751357c9b4512abcfc4d10afbb4
Reviewed-by: Pavel Dubsky <pavel.dubsky@qt.io>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>

5 files changed, 150 insertions, 36 deletions

diff --git a/src/activeqt/axbase/CMakeLists.txt b/src/activeqt/axbase/CMakeLists.txt
index aba9ebc..9d6ea74 100644
--- a/src/activeqt/axbase/CMakeLists.txt
+++ b/src/activeqt/axbase/CMakeLists.txt
@@ -11,6 +11,7 @@ qt_internal_add_module(AxBasePrivate
     SOURCES
         qaxtypefunctions.cpp qaxtypefunctions_p.h
         qaxutils.cpp qaxutils_p.h
+        qbstr_p.h
     LIBRARIES
         advapi32
         gdi32
diff --git a/src/activeqt/axbase/qaxutils.cpp b/src/activeqt/axbase/qaxutils.cpp
index f93910a..1645991 100644
--- a/src/activeqt/axbase/qaxutils.cpp
+++ b/src/activeqt/axbase/qaxutils.cpp
@@ -17,6 +17,8 @@
 
 #include <QtCore/qdebug.h>
 
+#include "qbstr_p.h"
+
 QT_BEGIN_NAMESPACE
 
 static inline QWindow *windowForWidget(QWidget *widget)
@@ -185,28 +187,25 @@ HRGN qaxHrgnFromQRegion(const QRegion &region, const QWidget *widget)
 QByteArray qaxTypeInfoName(ITypeInfo *typeInfo, MEMBERID memId)
 {
     QByteArray result;
-    BSTR names;
+    QBStr names;
     UINT cNames = 0;
     typeInfo->GetNames(memId, &names, 1, &cNames);
-    if (cNames && names) {
-        result = QString::fromWCharArray(names).toLatin1();
-        SysFreeString(names);
-    }
+    if (cNames && names)
+        result = names.str().toLatin1();
+
     return result;
 }
 
 QByteArrayList qaxTypeInfoNames(ITypeInfo *typeInfo, MEMBERID memId)
 {
     QByteArrayList result;
-    BSTR bstrNames[256];
+    QBStr bstrNames[256];
     UINT maxNames = 255;
     UINT maxNamesOut = 0;
     typeInfo->GetNames(memId, reinterpret_cast<BSTR *>(&bstrNames), maxNames, &maxNamesOut);
     result.reserve(maxNamesOut);
-    for (UINT p = 0; p < maxNamesOut; ++p) {
-        result.append(QString::fromWCharArray(bstrNames[p]).toLatin1());
-        SysFreeString(bstrNames[p]);
-    }
+    for (UINT p = 0; p < maxNamesOut; ++p)
+        result.append(bstrNames[p].str().toLatin1());
     return result;
 }
 
diff --git a/src/activeqt/axbase/qbstr_p.h b/src/activeqt/axbase/qbstr_p.h
new file mode 100644
index 0000000..115e3c3
--- /dev/null
+++ b/src/activeqt/axbase/qbstr_p.h
@@ -0,0 +1,119 @@
+// 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/QtGlobal>
+#include <QtCore/QString>
+#include <utility>
+#include <oaidl.h>
+
+class QBStr
+{
+public:
+    QBStr() = default;
+
+    ~QBStr() noexcept
+    {
+        free();
+    }
+
+    explicit QBStr(LPCOLESTR str) noexcept
+    {
+        if (str)
+            m_str = ::SysAllocString(str);
+        Q_ASSERT(m_str);
+    }
+
+    explicit QBStr(const QString &str) noexcept
+    {
+        m_str = ::SysAllocStringLen(reinterpret_cast<const OLECHAR *>(str.unicode()),
+                                    static_cast<UINT>(str.length()));
+        Q_ASSERT(m_str);
+    }
+
+    QBStr(const QBStr &str) noexcept : m_str{ str.copy() } { }
+
+    QBStr(QBStr &&str) noexcept : m_str{ std::exchange(str.m_str, nullptr) } { }
+
+    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;
+    }
+
+    explicit operator bool() const noexcept
+    {
+        return m_str != nullptr;
+    }
+
+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;
+};
+
+#endif
diff --git a/src/activeqt/container/qaxbase.cpp b/src/activeqt/container/qaxbase.cpp
index 3463f3c..87a7930 100644
--- a/src/activeqt/container/qaxbase.cpp
+++ b/src/activeqt/container/qaxbase.cpp
@@ -32,7 +32,7 @@
 #include <private/qmetaobject_p.h>
 #include <private/qmetaobjectbuilder_p.h>
 #include <private/qtools_p.h>
-
+#include <private/qbstr_p.h>
 #include <qt_windows.h>
 #include <ocidl.h>
 #include <ctype.h>
@@ -1830,16 +1830,14 @@ QByteArray MetaObjectGenerator::usertypeToString(const TYPEDESC &tdesc, ITypeInf
         usertypeinfo->GetContainingTypeLib(&usertypelib, &index);
         if (usertypelib) {
             // get type library name
-            BSTR typelibname = nullptr;
+            QBStr typelibname;
             usertypelib->GetDocumentation(-1, &typelibname, nullptr, nullptr, nullptr);
-            QByteArray typeLibName = QString::fromWCharArray(typelibname).toLatin1();
-            SysFreeString(typelibname);
+            QByteArray typeLibName = typelibname.str().toLatin1();
 
             // get type name
-            BSTR usertypename = nullptr;
+            QBStr usertypename;
             usertypelib->GetDocumentation(INT(index), &usertypename, nullptr, nullptr, nullptr);
-            QByteArray userTypeName = QString::fromWCharArray(usertypename).toLatin1();
-            SysFreeString(usertypename);
+            QByteArray userTypeName = usertypename.str().toLatin1();
 
             if (hasEnum(userTypeName)) // known enum?
                 typeName = userTypeName;
@@ -2233,14 +2231,12 @@ void MetaObjectGenerator::readEnumInfo()
                 continue;
 
             // Get the name of the enumeration
-            BSTR enumname;
+            QBStr enumname;
             QByteArray enumName;
-            if (typelib->GetDocumentation(INT(i), &enumname, nullptr, nullptr, nullptr) == S_OK) {
-                enumName = QString::fromWCharArray(enumname).toLatin1();
-                SysFreeString(enumname);
-            } else {
+            if (typelib->GetDocumentation(INT(i), &enumname, nullptr, nullptr, nullptr) == S_OK)
+                enumName = enumname.str().toLatin1();
+            else
                 enumName = "enum" + QByteArray::number(++enum_serial);
-            }
 
             // Get the attributes of the enum type
             for (const auto &value : values) {
@@ -2949,10 +2945,9 @@ QMetaObject *MetaObjectGenerator::metaObject(const QMetaObject *parentObject, co
     if (that) {
         readClassInfo();
         if (typelib) {
-            BSTR bstr;
+            QBStr bstr;
             typelib->GetDocumentation(-1, &bstr, nullptr, nullptr, nullptr);
-            current_typelib = QString::fromWCharArray(bstr).toLatin1();
-            SysFreeString(bstr);
+            current_typelib = bstr.str().toLatin1();
         }
         if (d->tryCache && tryCache())
             return d->metaobj;
diff --git a/src/activeqt/container/qaxscript.cpp b/src/activeqt/container/qaxscript.cpp
index cc5da7d..8ca6cad 100644
--- a/src/activeqt/container/qaxscript.cpp
+++ b/src/activeqt/container/qaxscript.cpp
@@ -18,7 +18,7 @@
 #include <quuid.h>
 #include <qwidget.h>
 #include <qlist.h>
-
+#include <QtAxBase/private/qbstr_p.h>
 #include <qt_windows.h>
 #ifndef QT_NO_QAXSCRIPT
 #include <initguid.h>
@@ -246,16 +246,15 @@ HRESULT WINAPI QAxScriptSite::OnScriptError(IActiveScriptError *error)
     DWORD context;
     ULONG lineNumber;
     LONG charPos;
-    BSTR bstrLineText;
+    QBStr bstrLineText;
     QString lineText;
 
     error->GetExceptionInfo(&exception);
     error->GetSourcePosition(&context, &lineNumber, &charPos);
     HRESULT hres = error->GetSourceLineText(&bstrLineText);
-    if (hres == S_OK) {
-        lineText = QString::fromWCharArray(bstrLineText);
-        SysFreeString(bstrLineText);
-    }
+    if (hres == S_OK)
+        lineText = bstrLineText.str();
+
     SysFreeString(exception.bstrSource);
     SysFreeString(exception.bstrDescription);
     SysFreeString(exception.bstrHelpFile);
@@ -463,13 +462,14 @@ bool QAxScriptEngine::initialize(IUnknown **ptr)
         return false;
     }
 
-    BSTR bstrCode = QStringToBSTR(script_code->scriptCode());
+    QBStr bstrCode{ script_code->scriptCode() };
 #ifdef Q_OS_WIN64
-    hres = parser->ParseScriptText(bstrCode, nullptr, nullptr, nullptr, DWORDLONG(this), 0, SCRIPTTEXT_ISVISIBLE, nullptr, nullptr);
+    hres = parser->ParseScriptText(bstrCode.bstr(), nullptr, nullptr, nullptr, DWORDLONG(this), 0,
+                                   SCRIPTTEXT_ISVISIBLE, nullptr, nullptr);
 #else
-    hres = parser->ParseScriptText(bstrCode, 0, 0, 0, DWORD(this), 0, SCRIPTTEXT_ISVISIBLE, 0, 0);
+    hres = parser->ParseScriptText(bstrCode.bstr(), 0, 0, 0, DWORD(this), 0, SCRIPTTEXT_ISVISIBLE,
+                                   0, 0);
 #endif
-    SysFreeString(bstrCode);
 
     parser->Release();
     parser = nullptr;