From 2596a8a55ce3c9532701c2cf1f353b93df4bd893 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Mon, 27 May 2019 16:56:12 +0200
Subject: [PATCH] QSharedNetworkSessionManager: clean up
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Under no circumstance must Qt code define a non-public qHash()
overload of a public Qt type. If you can't provide a public qHash()
for the type, you can't use QHash. Period. This is because if we don't
provide a qHash() function, the user will have to provide one, and
then we have an ODR violation, iow: UB.

So, port away from QHash and qHash and use std::unordered_map, which
allows to pass a custom hash function - just what we needed.

Also fix other issues of the code:

- Use a function object as the deleter of the QSharedPointer, to allow
  inlining the function call.

- Avoid double lookup in case of a cache miss.

- Don't use direct initialization syntax for pointer variables. That's
  just ... weird.

Change-Id: Ica8bff728c1bd1cbb2fb43aa03a99bcb7ac2f7cc
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/network/bearer/qsharednetworksession.cpp | 34 ++++++++------------
 src/network/bearer/qsharednetworksession_p.h | 16 +++++++--
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/src/network/bearer/qsharednetworksession.cpp b/src/network/bearer/qsharednetworksession.cpp
index fc01acb8b4..af543f77e3 100644
--- a/src/network/bearer/qsharednetworksession.cpp
+++ b/src/network/bearer/qsharednetworksession.cpp
@@ -57,36 +57,30 @@ inline QSharedNetworkSessionManager* sharedNetworkSessionManager()
     return rv;
 }
 
-static void doDeleteLater(QObject* obj)
-{
-    obj->deleteLater();
-}
+struct DeleteLater {
+    void operator()(QObject* obj) const
+    {
+        obj->deleteLater();
+    }
+};
 
 QSharedPointer<QNetworkSession> QSharedNetworkSessionManager::getSession(const QNetworkConfiguration &config)
 {
-    QSharedNetworkSessionManager *m(sharedNetworkSessionManager());
-    const auto it = m->sessions.constFind(config);
+    QSharedNetworkSessionManager *m = sharedNetworkSessionManager();
+    auto &entry = m->sessions[config];
     //if already have a session, return it
-    if (it != m->sessions.cend()) {
-        QSharedPointer<QNetworkSession> p = it.value().toStrongRef();
-        if (!p.isNull())
-            return p;
-    }
+    if (auto p = entry.toStrongRef())
+        return p;
     //otherwise make one
-    QSharedPointer<QNetworkSession> session(new QNetworkSession(config), doDeleteLater);
-    m->sessions[config] = session;
+    QSharedPointer<QNetworkSession> session(new QNetworkSession(config), DeleteLater{});
+    entry = session;
     return session;
 }
 
 void QSharedNetworkSessionManager::setSession(const QNetworkConfiguration &config, QSharedPointer<QNetworkSession> session)
 {
-    QSharedNetworkSessionManager *m(sharedNetworkSessionManager());
-    m->sessions[config] = session;
-}
-
-uint qHash(const QNetworkConfiguration& config)
-{
-    return ((uint)config.type()) | (((uint)config.bearerType()) << 8) | (((uint)config.purpose()) << 16);
+    QSharedNetworkSessionManager *m = sharedNetworkSessionManager();
+    m->sessions[config] = std::move(session);
 }
 
 QT_END_NAMESPACE
diff --git a/src/network/bearer/qsharednetworksession_p.h b/src/network/bearer/qsharednetworksession_p.h
index 001b8af02a..f22f9eeacb 100644
--- a/src/network/bearer/qsharednetworksession_p.h
+++ b/src/network/bearer/qsharednetworksession_p.h
@@ -54,16 +54,26 @@
 #include <QtNetwork/private/qtnetworkglobal_p.h>
 #include "qnetworksession.h"
 #include "qnetworkconfiguration.h"
-#include <QHash>
 #include <QSharedPointer>
 #include <QWeakPointer>
 #include <QMutex>
 
+#include <unordered_map>
+
 #ifndef QT_NO_BEARERMANAGEMENT
 
 QT_BEGIN_NAMESPACE
 
-uint qHash(const QNetworkConfiguration& config);
+namespace QtPrivate {
+struct NetworkConfigurationHash {
+    using result_type = size_t;
+    using argument_type = QNetworkConfiguration;
+    size_t operator()(const QNetworkConfiguration &config) const noexcept
+    {
+        return std::hash<size_t>{}(size_t(config.type()) | (size_t(config.bearerType()) << 8) | (size_t(config.purpose()) << 16));
+    }
+};
+}
 
 class QSharedNetworkSessionManager
 {
@@ -71,7 +81,7 @@ public:
     static QSharedPointer<QNetworkSession> getSession(const QNetworkConfiguration &config);
     static void setSession(const QNetworkConfiguration &config, QSharedPointer<QNetworkSession> session);
 private:
-    QHash<QNetworkConfiguration, QWeakPointer<QNetworkSession> > sessions;
+    std::unordered_map<QNetworkConfiguration, QWeakPointer<QNetworkSession>, QtPrivate::NetworkConfigurationHash> sessions;
 };
 
 QT_END_NAMESPACE