Turn raw QSettings pointer into a reference in the CredentialManagerwork/change-ptr-to-ref

The underlying object is managed by a std::unique_ptr. So accidentally
setting a parent, or managing it somewhere else, will result in a
double-free. Returning a reference is slightly more safe, and has the
added benefit that it communicates the fact that it will never be a
nullptr.

2 files changed, 15 insertions, 10 deletions

diff --git a/src/libsync/creds/credentialmanager.cpp b/src/libsync/creds/credentialmanager.cpp
index 9f22e55ba..9ab85d27c 100644
--- a/src/libsync/creds/credentialmanager.cpp
+++ b/src/libsync/creds/credentialmanager.cpp
@@ -66,7 +66,7 @@ QKeychain::Job *CredentialManager::set(const QString &key, const QVariant &data)
         if (writeJob->error() == QKeychain::NoError) {
             qCInfo(lcCredentaislManager) << "added" << scopedKey(this, key);
             // just a list, the values don't matter
-            credentialsList()->setValue(key, true);
+            credentialsList().setValue(key, true);
         } else {
             qCWarning(lcCredentaislManager) << "Failed to set:" << scopedKey(this, key) << writeJob->errorString();
         }
@@ -81,7 +81,7 @@ QKeychain::Job *CredentialManager::remove(const QString &key)
 {
     OC_ASSERT(contains(key));
     // remove immediately to prevent double invocation by clear()
-    credentialsList()->remove(key);
+    credentialsList().remove(key);
     qCInfo(lcCredentaislManager) << "del" << scopedKey(this, key);
     auto keychainJob = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
     keychainJob->setKey(scopedKey(this, key));
@@ -117,32 +117,37 @@ const Account *CredentialManager::account() const
 
 bool CredentialManager::contains(const QString &key) const
 {
-    return credentialsList()->contains(key);
+    return credentialsList().contains(key);
 }
 
 QStringList CredentialManager::knownKeys(const QString &group) const
 {
     if (group.isEmpty()) {
-        return credentialsList()->allKeys();
+        return credentialsList().allKeys();
     }
-    credentialsList()->beginGroup(group);
-    const auto keys = credentialsList()->allKeys();
+    credentialsList().beginGroup(group);
+    const auto keys = credentialsList().allKeys();
     QStringList out;
     out.reserve(keys.size());
     for (const auto &k : keys) {
         out.append(group + QLatin1Char('/') + k);
     }
-    credentialsList()->endGroup();
+    credentialsList().endGroup();
     return out;
 }
 
-QSettings *CredentialManager::credentialsList() const
+/**
+ * Utility function to lazily create the settings (group).
+ *
+ * IMPORTANT: the underlying storage is a std::unique_ptr, so do *NOT* store this reference anywhere!
+ */
+QSettings &CredentialManager::credentialsList() const
 {
     // delayed init as scope requires a fully inizialised acc
     if (!_credentialsList) {
         _credentialsList = ConfigFile::settingsWithGroup(QStringLiteral("Credentials/") + scope(this));
     }
-    return _credentialsList.get();
+    return *_credentialsList;
 }
 
 CredentialJob::CredentialJob(CredentialManager *parent, const QString &key)
diff --git a/src/libsync/creds/credentialmanager.h b/src/libsync/creds/credentialmanager.h
index 07e961c63..e9b8fa88d 100644
--- a/src/libsync/creds/credentialmanager.h
+++ b/src/libsync/creds/credentialmanager.h
@@ -34,7 +34,7 @@ public:
     const Account *account() const;
 
 private:
-    QSettings *credentialsList() const;
+    QSettings &credentialsList() const;
 
     // TestCredentialManager
     QStringList knownKeys(const QString &group = {}) const;