QWinRegistryKey: refactor

Refactor QWinRegistryKey to make it more modern and more
usable from outside.

Adjust the test for QWinRegistryKey to test the new functions,
merged with the original test.

Will port raw registry accessing code in QtBase to use this
class in follow-up commits. This change is the first step.

The long term goal is to port QSettings registry code to
this class instead of using raw Win32 APIs, however, there's
much more registry code in QSettings and migrate them to this
class needs a large refactor, so jsut leave it for now. Will
fix it in some future commit.

Change-Id: Iada2adb41b4d58e1b658ff6870a4b31ace479d43
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

9 files changed, 350 insertions, 104 deletions

diff --git a/src/corelib/kernel/qwinregistry.cpp b/src/corelib/kernel/qwinregistry.cpp
index 368ae2faad..c31b1b1549 100644
--- a/src/corelib/kernel/qwinregistry.cpp
+++ b/src/corelib/kernel/qwinregistry.cpp
@@ -2,15 +2,12 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
 #include "qwinregistry_p.h"
-
 #include <QtCore/qvarlengtharray.h>
-
-#include <algorithm>
+#include <QtCore/qendian.h>
 
 QT_BEGIN_NAMESPACE
 
-QWinRegistryKey::QWinRegistryKey() :
-    m_key(nullptr)
+QWinRegistryKey::QWinRegistryKey()
 {
 }
 
@@ -19,8 +16,8 @@ QWinRegistryKey::QWinRegistryKey() :
 QWinRegistryKey::QWinRegistryKey(HKEY parentHandle, QStringView subKey,
                                  REGSAM permissions, REGSAM access)
 {
-    if (RegOpenKeyEx(parentHandle, reinterpret_cast<const wchar_t *>(subKey.utf16()),
-                     0, permissions | access, &m_key) != ERROR_SUCCESS) {
+    if (RegOpenKeyExW(parentHandle, reinterpret_cast<const wchar_t *>(subKey.utf16()),
+                      0, permissions | access, &m_key) != ERROR_SUCCESS) {
         m_key = nullptr;
     }
 }
@@ -38,45 +35,97 @@ void QWinRegistryKey::close()
     }
 }
 
-QString QWinRegistryKey::stringValue(QStringView subKey) const
+QVariant QWinRegistryKey::value(QStringView subKey) const
 {
-    QString result;
+    Q_ASSERT(!subKey.isEmpty());
+
     if (!isValid())
-        return result;
-    DWORD type;
-    DWORD size;
+        return {};
+
     auto subKeyC = reinterpret_cast<const wchar_t *>(subKey.utf16());
-    if (RegQueryValueEx(m_key, subKeyC, nullptr, &type, nullptr, &size) != ERROR_SUCCESS
-        || (type != REG_SZ && type != REG_EXPAND_SZ) || size <= 2) {
-        return result;
+
+    // Get the size and type of the value.
+    DWORD dataType = REG_NONE;
+    DWORD dataSize = 0;
+    LONG ret = RegQueryValueExW(m_key, subKeyC, nullptr, &dataType, nullptr, &dataSize);
+    if (ret != ERROR_SUCCESS)
+        return {};
+
+    // Workaround for rare cases where the trailing '\0' is missing.
+    if (dataType == REG_SZ || dataType == REG_EXPAND_SZ)
+        dataSize += 2;
+    else if (dataType == REG_MULTI_SZ)
+        dataSize += 4;
+
+    // Get the value.
+    QVarLengthArray<unsigned char> data(dataSize);
+    std::fill(data.data(), data.data() + dataSize, 0u);
+
+    ret = RegQueryValueExW(m_key, subKeyC, nullptr, nullptr, data.data(), &dataSize);
+    if (ret != ERROR_SUCCESS)
+        return {};
+
+    switch (dataType) {
+        case REG_SZ:
+        case REG_EXPAND_SZ: {
+            if (dataSize > 0) {
+                return QString::fromWCharArray(
+                    reinterpret_cast<const wchar_t *>(data.constData()));
+            }
+            return QString();
+        }
+
+        case REG_MULTI_SZ: {
+            if (dataSize > 0) {
+                QStringList list = {};
+                int i = 0;
+                while (true) {
+                    const QString str = QString::fromWCharArray(
+                        reinterpret_cast<const wchar_t *>(data.constData()) + i);
+                    i += str.length() + 1;
+                    if (str.isEmpty())
+                        break;
+                    list.append(str);
+                }
+                return list;
+            }
+            return QStringList();
+        }
+
+        case REG_NONE: // No specific type, treat as binary data.
+        case REG_BINARY: {
+            if (dataSize > 0) {
+                return QString::fromWCharArray(
+                    reinterpret_cast<const wchar_t *>(data.constData()), data.size() / 2);
+            }
+            return QString();
+        }
+
+        case REG_DWORD: // Same as REG_DWORD_LITTLE_ENDIAN
+            return qFromLittleEndian<quint32>(data.constData());
+
+        case REG_DWORD_BIG_ENDIAN:
+            return qFromBigEndian<quint32>(data.constData());
+
+        case REG_QWORD: // Same as REG_QWORD_LITTLE_ENDIAN
+            return qFromLittleEndian<quint64>(data.constData());
+
+        default:
+            break;
     }
-    // Reserve more for rare cases where trailing '\0' are missing in registry.
-    // Rely on 0-termination since strings of size 256 padded with 0 have been
-    // observed (QTBUG-84455).
-    size += 2;
-    QVarLengthArray<unsigned char> buffer(static_cast<int>(size));
-    std::fill(buffer.data(), buffer.data() + size, 0u);
-    if (RegQueryValueEx(m_key, subKeyC, nullptr, &type, buffer.data(), &size) == ERROR_SUCCESS)
-          result = QString::fromWCharArray(reinterpret_cast<const wchar_t *>(buffer.constData()));
-    return result;
+
+    return {};
+}
+
+QString QWinRegistryKey::stringValue(QStringView subKey) const
+{
+    return value<QString>(subKey).value_or(QString());
 }
 
 QPair<DWORD, bool> QWinRegistryKey::dwordValue(QStringView subKey) const
 {
-    if (!isValid())
-        return qMakePair(0, false);
-    DWORD type;
-    auto subKeyC = reinterpret_cast<const wchar_t *>(subKey.utf16());
-    if (RegQueryValueEx(m_key, subKeyC, nullptr, &type, nullptr, nullptr) != ERROR_SUCCESS
-        || type != REG_DWORD) {
-        return qMakePair(0, false);
-    }
-    DWORD value = 0;
-    DWORD size = sizeof(value);
-    const bool ok =
-        RegQueryValueEx(m_key, subKeyC, nullptr, nullptr,
-                        reinterpret_cast<unsigned char *>(&value), &size) == ERROR_SUCCESS;
-    return qMakePair(value, ok);
+    const std::optional<DWORD> val = value<DWORD>(subKey);
+    return qMakePair(val.value_or(0), val.has_value());
 }
 
 QT_END_NAMESPACE
diff --git a/src/corelib/kernel/qwinregistry_p.h b/src/corelib/kernel/qwinregistry_p.h
index 0dfa6c70fb..888f8e0ce5 100644
--- a/src/corelib/kernel/qwinregistry_p.h
+++ b/src/corelib/kernel/qwinregistry_p.h
@@ -19,15 +19,15 @@
 #include <QtCore/qstring.h>
 #include <QtCore/qstringview.h>
 #include <QtCore/qt_windows.h>
-#include <QtCore/private/qglobal_p.h>
+#include <QtCore/qvariant.h>
 
 QT_BEGIN_NAMESPACE
 
 class Q_CORE_EXPORT QWinRegistryKey
 {
-public:
     Q_DISABLE_COPY(QWinRegistryKey)
 
+public:
     QWinRegistryKey();
     explicit QWinRegistryKey(HKEY parentHandle, QStringView subKey,
                              REGSAM permissions = KEY_READ, REGSAM access = 0);
@@ -38,15 +38,30 @@ public:
     QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_MOVE_AND_SWAP(QWinRegistryKey)
     void swap(QWinRegistryKey &other) noexcept { qSwap(m_key, other.m_key); }
 
-    bool isValid() const { return m_key != nullptr; }
-    operator HKEY() const { return m_key; }
+    [[nodiscard]] bool isValid() const { return m_key != nullptr; }
+
+    [[nodiscard]] HKEY handle() const { return m_key; }
+
+    operator HKEY() const { return handle(); }
+
     void close();
 
+    [[nodiscard]] QVariant value(QStringView subKey) const;
+    template<typename T>
+    [[nodiscard]] std::optional<T> value(QStringView subKey) const
+    {
+        const QVariant var = value(subKey);
+        if (var.isValid())
+            return qvariant_cast<T>(var);
+        return std::nullopt;
+    }
+
+    // ### TODO: Remove once all usages are migrated to new interface.
     QString stringValue(QStringView subKey) const;
     QPair<DWORD, bool> dwordValue(QStringView subKey) const;
 
 private:
-    HKEY m_key;
+    HKEY m_key = nullptr;
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/corelib/global/CMakeLists.txt b/tests/auto/corelib/global/CMakeLists.txt
index 621c3d723b..6ccf3c2ed0 100644
--- a/tests/auto/corelib/global/CMakeLists.txt
+++ b/tests/auto/corelib/global/CMakeLists.txt
@@ -21,7 +21,4 @@ add_subdirectory(qtendian)
 add_subdirectory(qglobalstatic)
 add_subdirectory(qhooks)
 add_subdirectory(qoperatingsystemversion)
-if(WIN32)
-    add_subdirectory(qwinregistry)
-endif()
 add_subdirectory(qxp)
diff --git a/tests/auto/corelib/global/qwinregistry/CMakeLists.txt b/tests/auto/corelib/global/qwinregistry/CMakeLists.txt
deleted file mode 100644
index 2f4613c79e..0000000000
--- a/tests/auto/corelib/global/qwinregistry/CMakeLists.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-# Copyright (C) 2022 The Qt Company Ltd.
-# SPDX-License-Identifier: BSD-3-Clause
-
-# Generated from qwinregistry.pro.
-
-#####################################################################
-## tst_qwinregistry Test:
-#####################################################################
-
-qt_internal_add_test(tst_qwinregistry
-    SOURCES
-        tst_qwinregistry.cpp
-    LIBRARIES
-        Qt::CorePrivate
-)
diff --git a/tests/auto/corelib/global/qwinregistry/tst_qwinregistry.cpp b/tests/auto/corelib/global/qwinregistry/tst_qwinregistry.cpp
deleted file mode 100644
index 423c4ebc82..0000000000
--- a/tests/auto/corelib/global/qwinregistry/tst_qwinregistry.cpp
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright (C) 2019 The Qt Company Ltd.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
-
-#include <QTest>
-
-#include <QtCore/qoperatingsystemversion.h>
-
-#include <QtCore/private/qwinregistry_p.h>
-
-class tst_QWinRegistry : public QObject
-{
-    Q_OBJECT
-
-public Q_SLOTS:
-    void initTestCase();
-
-private Q_SLOTS:
-    void values();
-};
-
-void tst_QWinRegistry::initTestCase()
-{
-    if (QOperatingSystemVersion::current() < QOperatingSystemVersion::Windows10)
-        QSKIP("This test requires registry values present in Windows 10");
-}
-
-void tst_QWinRegistry::values()
-{
-    QWinRegistryKey key(HKEY_LOCAL_MACHINE, LR"(SOFTWARE\Microsoft\Windows NT\CurrentVersion)");
-    QVERIFY(key.isValid());
-    QVERIFY(!key.stringValue(L"ProductName").isEmpty());
-    QVERIFY(key.stringValue(L"NonExistingKey").isEmpty());
-    auto majorVersion = key.dwordValue(L"CurrentMajorVersionNumber");
-    QVERIFY(majorVersion.second);
-    QVERIFY(majorVersion.first > 0);
-    auto nonExistingValue = key.dwordValue(L"NonExistingKey");
-    QVERIFY(!nonExistingValue.second);
-    QCOMPARE(nonExistingValue.first, 0u);
-}
-
-QTEST_APPLESS_MAIN(tst_QWinRegistry);
-
-#include "tst_qwinregistry.moc"
diff --git a/tests/auto/corelib/kernel/CMakeLists.txt b/tests/auto/corelib/kernel/CMakeLists.txt
index 1b8625e90b..a556b09953 100644
--- a/tests/auto/corelib/kernel/CMakeLists.txt
+++ b/tests/auto/corelib/kernel/CMakeLists.txt
@@ -45,6 +45,7 @@ if(QT_FEATURE_systemsemaphore AND NOT ANDROID AND NOT UIKIT)
 endif()
 if(WIN32)
     add_subdirectory(qwineventnotifier)
+    add_subdirectory(qwinregistrykey)
 endif()
 if(QT_FEATURE_private_tests)
     add_subdirectory(qproperty)
diff --git a/tests/auto/corelib/kernel/qwinregistrykey/.gitignore b/tests/auto/corelib/kernel/qwinregistrykey/.gitignore
new file mode 100644
index 0000000000..3d888e2868
--- /dev/null
+++ b/tests/auto/corelib/kernel/qwinregistrykey/.gitignore
@@ -0,0 +1 @@
+tst_qwinregistrykey
diff --git a/tests/auto/corelib/kernel/qwinregistrykey/CMakeLists.txt b/tests/auto/corelib/kernel/qwinregistrykey/CMakeLists.txt
new file mode 100644
index 0000000000..762debd51f
--- /dev/null
+++ b/tests/auto/corelib/kernel/qwinregistrykey/CMakeLists.txt
@@ -0,0 +1,9 @@
+# Copyright (C) 2022 The Qt Company Ltd.
+# SPDX-License-Identifier: BSD-3-Clause
+
+qt_internal_add_test(tst_qwinregistrykey
+    SOURCES
+        tst_qwinregistrykey.cpp
+    LIBRARIES
+        Qt::CorePrivate
+)
diff --git a/tests/auto/corelib/kernel/qwinregistrykey/tst_qwinregistrykey.cpp b/tests/auto/corelib/kernel/qwinregistrykey/tst_qwinregistrykey.cpp
new file mode 100644
index 0000000000..d3a20be048
--- /dev/null
+++ b/tests/auto/corelib/kernel/qwinregistrykey/tst_qwinregistrykey.cpp
@@ -0,0 +1,232 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include <QTest>
+#include <QObject>
+#include <QPair>
+#include <QScopeGuard>
+#include <private/qwinregistry_p.h>
+#include <qt_windows.h>
+
+using namespace Qt::StringLiterals;
+
+static constexpr const wchar_t TEST_KEY[] = LR"(SOFTWARE\tst_qwinregistrykey)";
+
+static const QPair<QStringView, QString> TEST_STRING = qMakePair(u"string", u"string"_s);
+static const QPair<QStringView, QString> TEST_STRING_NULL = qMakePair(u"string_null", QString());
+static const QPair<QStringView, QStringList> TEST_STRINGLIST = qMakePair(u"stringlist", QStringList{ u"element1"_s, u"element2"_s, u"element3"_s });
+static const QPair<QStringView, QStringList> TEST_STRINGLIST_NULL = qMakePair(u"stringlist_null", QStringList());
+static const QPair<QStringView, quint32> TEST_DWORD = qMakePair(u"dword", 123);
+static const QPair<QStringView, quint64> TEST_QWORD = qMakePair(u"qword", 456);
+static const QPair<QStringView, QByteArray> TEST_BINARY = qMakePair(u"binary", "binary\0"_ba);
+static const QPair<QStringView, QVariant> TEST_NOT_EXIST = qMakePair(u"not_exist", QVariant());
+
+[[nodiscard]] static inline bool write(const HKEY key, const QStringView name, const QVariant &value)
+{
+    DWORD type = REG_NONE;
+    QByteArray buf = {};
+
+    switch (value.typeId()) {
+        case QMetaType::QStringList: {
+            // If none of the elements contains '\0', we can use REG_MULTI_SZ, the
+            // native registry string list type. Otherwise we use REG_BINARY.
+            type = REG_MULTI_SZ;
+            const QStringList list = value.toStringList();
+            for (auto it = list.constBegin(); it != list.constEnd(); ++it) {
+                if ((*it).length() == 0 || it->contains(QChar::Null)) {
+                    type = REG_BINARY;
+                    break;
+                }
+            }
+
+            if (type == REG_BINARY) {
+                const QString str = value.toString();
+                buf = QByteArray(reinterpret_cast<const char *>(str.data()), str.length() * 2);
+            } else {
+                for (auto it = list.constBegin(); it != list.constEnd(); ++it) {
+                    const QString &str = *it;
+                    buf += QByteArray(reinterpret_cast<const char *>(str.utf16()), (str.length() + 1) * 2);
+                }
+                // According to Microsoft Docs, REG_MULTI_SZ requires double '\0'.
+                buf.append((char)0);
+                buf.append((char)0);
+            }
+            break;
+        }
+
+        case QMetaType::Int:
+        case QMetaType::UInt: {
+            type = REG_DWORD;
+            quint32 num = value.toUInt();
+            buf = QByteArray(reinterpret_cast<const char *>(&num), sizeof(quint32));
+            break;
+        }
+
+        case QMetaType::LongLong:
+        case QMetaType::ULongLong: {
+            type = REG_QWORD;
+            quint64 num = value.toULongLong();
+            buf = QByteArray(reinterpret_cast<const char *>(&num), sizeof(quint64));
+            break;
+        }
+
+        case QMetaType::QByteArray:
+        default: {
+            // If the string does not contain '\0', we can use REG_SZ, the native registry
+            // string type. Otherwise we use REG_BINARY.
+            const QString str = value.toString();
+            type = str.contains(QChar::Null) ? REG_BINARY : REG_SZ;
+            int length = str.length();
+            if (type == REG_SZ)
+                ++length;
+            buf = QByteArray(reinterpret_cast<const char *>(str.utf16()), sizeof(wchar_t) * length);
+            break;
+        }
+    }
+
+    const LONG ret = RegSetValueExW(key, reinterpret_cast<const wchar_t *>(name.utf16()),
+                                    0, type, reinterpret_cast<LPBYTE>(buf.data()), buf.size());
+    return ret == ERROR_SUCCESS;
+}
+
+class tst_qwinregistrykey : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void initTestCase();
+    void cleanupTestCase();
+    void qwinregistrykey();
+
+private:
+    bool m_available = false;
+};
+
+void tst_qwinregistrykey::initTestCase()
+{
+    HKEY key = nullptr;
+    const LONG ret = RegCreateKeyExW(HKEY_CURRENT_USER, TEST_KEY, 0, nullptr, 0,
+                                     KEY_READ | KEY_WRITE, nullptr, &key, nullptr);
+    if (ret != ERROR_SUCCESS)
+        return;
+    const auto cleanup = qScopeGuard([key](){ RegCloseKey(key); });
+    if (!write(key, TEST_STRING.first, TEST_STRING.second))
+        return;
+    if (!write(key, TEST_STRING_NULL.first, TEST_STRING_NULL.second))
+        return;
+    if (!write(key, TEST_STRINGLIST.first, TEST_STRINGLIST.second))
+        return;
+    if (!write(key, TEST_STRINGLIST_NULL.first, TEST_STRINGLIST_NULL.second))
+        return;
+    if (!write(key, TEST_DWORD.first, TEST_DWORD.second))
+        return;
+    if (!write(key, TEST_QWORD.first, TEST_QWORD.second))
+        return;
+    if (!write(key, TEST_BINARY.first, TEST_BINARY.second))
+        return;
+    m_available = true;
+}
+
+void tst_qwinregistrykey::cleanupTestCase()
+{
+    HKEY key = nullptr;
+    const LONG ret = RegOpenKeyExW(HKEY_CURRENT_USER, TEST_KEY, 0, KEY_READ | KEY_WRITE, &key);
+    if (ret != ERROR_SUCCESS)
+        return;
+    #define C_STR(View) reinterpret_cast<const wchar_t *>(View.utf16())
+    RegDeleteValueW(key, C_STR(TEST_STRING.first));
+    RegDeleteValueW(key, C_STR(TEST_STRING_NULL.first));
+    RegDeleteValueW(key, C_STR(TEST_STRINGLIST.first));
+    RegDeleteValueW(key, C_STR(TEST_STRINGLIST_NULL.first));
+    RegDeleteValueW(key, C_STR(TEST_DWORD.first));
+    RegDeleteValueW(key, C_STR(TEST_QWORD.first));
+    RegDeleteValueW(key, C_STR(TEST_BINARY.first));
+    #undef C_STR
+    RegCloseKey(key);
+    RegDeleteKeyW(HKEY_CURRENT_USER, TEST_KEY);
+}
+
+void tst_qwinregistrykey::qwinregistrykey()
+{
+    if (!m_available)
+        QSKIP("The test data is not ready.");
+
+    QWinRegistryKey registry(HKEY_CURRENT_USER, TEST_KEY);
+
+    QVERIFY(registry.isValid());
+
+    QVERIFY(registry.handle() != nullptr);
+
+    {
+        const auto value = registry.value<QString>(TEST_STRING.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(QString()), TEST_STRING.second);
+    }
+
+    {
+        const auto value = registry.value<QString>(TEST_STRING_NULL.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(QString()), TEST_STRING_NULL.second);
+
+    }
+
+    {
+        const auto value = registry.value<QStringList>(TEST_STRINGLIST.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(QStringList()), TEST_STRINGLIST.second);
+    }
+
+    {
+        const auto value = registry.value<QStringList>(TEST_STRINGLIST_NULL.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(QStringList()), TEST_STRINGLIST_NULL.second);
+    }
+
+    {
+        const auto value = registry.value<quint32>(TEST_DWORD.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(0), TEST_DWORD.second);
+    }
+
+    {
+        const auto value = registry.value<quint64>(TEST_QWORD.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(0), TEST_QWORD.second);
+    }
+
+    {
+        const auto value = registry.value<QByteArray>(TEST_BINARY.first);
+        QVERIFY(value.has_value());
+        QCOMPARE(value.value_or(QByteArray()), TEST_BINARY.second);
+    }
+
+    {
+        const auto value = registry.value<QVariant>(TEST_NOT_EXIST.first);
+        QVERIFY(!value.has_value());
+        QCOMPARE(value.value_or(QVariant()), QVariant());
+    }
+
+    {
+        const QString value = registry.stringValue(TEST_STRING.first);
+        QVERIFY(!value.isEmpty());
+        QCOMPARE(value, TEST_STRING.second);
+    }
+
+    QVERIFY(registry.stringValue(TEST_NOT_EXIST.first).isEmpty());
+
+    {
+        const auto value = registry.dwordValue(TEST_DWORD.first);
+        QVERIFY(value.second);
+        QCOMPARE(value.first, TEST_DWORD.second);
+    }
+
+    {
+        const auto value = registry.dwordValue(TEST_NOT_EXIST.first);
+        QVERIFY(!value.second);
+        QCOMPARE(value.first, DWORD(0));
+    }
+}
+
+QTEST_MAIN(tst_qwinregistrykey)
+
+#include "tst_qwinregistrykey.moc"