Differential D13034 Diff 43304 src/core/kconfigwatcher.cpp

Changeset View

Standalone View

View Options

src/core/kconfigwatcher.cpp

  • This file was added.
1​/*
2​ * Copyright 2018 David Edmundson <davidedmundson@kde.org>
dfaureUnsubmitted
Done

License header missing

dfaure: License header missing
3​ *
4​ * This program is free software; you can redistribute it and/or modify
5​ * it under the terms of the GNU Library General Public License as
6​ * published by the Free Software Foundation; either version 2, or
7​ * (at your option) any later version.
8​ *
9​ * This program is distributed in the hope that it will be useful,
10​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
11​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12​ * GNU General Public License for more details
13​ *
14​ * You should have received a copy of the GNU Library General Public
15​ * License along with this program; if not, write to the
16​ * Free Software Foundation, Inc.,
17​ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
18​ */
19​
20​#include "kconfigwatcher.h"
21​
dfaureUnsubmitted
Done

(I'm curious, why not a raw pointer? You clean up the hash when the watcher is destroyed anyway; probably doesn't matter either way though)

dfaure: (I'm curious, why not a raw pointer? You clean up the hash when the watcher is destroyed anyway…
davidedmundsonAuthorUnsubmitted
Not Done

I'm returning QSharedPointers to the caller.

davidedmundson: I'm returning QSharedPointers to the caller.
22​#include "config-kconfig.h"
dfaureUnsubmitted
Done

You could also do this the C++11 way with a static object inside the create() method, which is the only place where it's used.

dfaure: You could also do this the C++11 way with a static object inside the create() method, which is…
23​
24​#if KCONFIG_USE_DBUS
25​#include <QDBusConnection>
26​#include <QDBusMessage>
27​#include <QDBusMetaType>
28​#endif
29​
dfaureUnsubmitted
Done

There should probably be a mutex around this whole method implementation, in case two threads create a watcher for the same KSharedConfig (which is itself threadsafe).

Alternatively, the static can be a QThreadStorage, like I did in ksharedconfig.cpp.

dfaure: There should probably be a mutex around this whole method implementation, in case two threads…
30​#include <QDebug>
31​#include <QThreadStorage>
32​#include <QHash>
33​
34​class KConfigWatcherPrivate {
35​public:
36​ KSharedConfig::Ptr m_config;
37​};
38​
39​KConfigWatcher::Ptr KConfigWatcher::create(const KSharedConfig::Ptr &config)
40​{
41​ static QThreadStorage<QHash<KSharedConfig*, QWeakPointer<KConfigWatcher>>> watcherList;
42​
43​ auto c = config.data();
44​ KConfigWatcher::Ptr watcher;
45​
46​ if (!watcherList.localData().contains(c)) {
47​ watcher = KConfigWatcher::Ptr(new KConfigWatcher(config));
48​
49​ watcherList.localData().insert(c, watcher.toWeakRef());
50​
51​ QObject::connect(watcher.data(), &QObject::destroyed, [c]() {
52​ watcherList.localData().remove(c);
53​ });
54​ }
55​ return watcherList.localData().value(c).toStrongRef();
56​}
57​
58​KConfigWatcher::KConfigWatcher(const KSharedConfig::Ptr &config):
59​ QObject (nullptr),
60​ d(new KConfigWatcherPrivate)
61​{
62​ Q_ASSERT(config);
zzagUnsubmitted
Done

I think you forgot qAsConst. ;-)

for (const QString &path : qAsConst(watchedPaths)) {
zzag: I think you forgot qAsConst. ;-) ```lang=cpp for (const QString &path : qAsConst…
63​#if KCONFIG_USE_DBUS
64​
65​ qDBusRegisterMetaType<QByteArrayList>();
66​ qDBusRegisterMetaType<QHash<QString, QByteArrayList>>();
67​
68​ d->m_config = config;
69​
70​ QStringList watchedPaths;
71​ watchedPaths <<QStringLiteral("/") + d->m_config->name();
72​ for (const QString file: d->m_config->additionalConfigSources()) {
73​ watchedPaths << QStringLiteral("/") + file;
dfaureUnsubmitted
Not Done

const QString &

and additionalConfigSources should be put into a local const var first, to avoid a detach.

dfaure: const QString & and additionalConfigSources should be put into a local const var first, to…
74​ }
75​ if (d->m_config->openFlags() & KConfig::IncludeGlobals) {
76​ watchedPaths << QStringLiteral("/kdeglobals");
77​ }
78​
79​ for(const QString &path: qAsConst(watchedPaths)) {
80​ QDBusConnection::sessionBus().connect(QString(),
81​ path,
82​ QStringLiteral("org.kde.kconfig.notify"),
83​ QStringLiteral("ConfigChanged"),
zzagUnsubmitted
Done

Won't this detach?

zzag: Won't this detach?
broulikUnsubmitted
Done

no, parts is const

broulik: no, `parts` is `const`
davidedmundsonAuthorUnsubmitted
Not Done

zzag was right then I changed it, but I hadn't clicked the "done" checkbox after fixing.

davidedmundson: zzag was right then I changed it, but I hadn't clicked the "done" checkbox after fixing.
84​ this,
85​ SLOT(onConfigChangeNotification(QHash<QString, QByteArrayList>)));
86​ }
87​#else
88​ qWarning() << "Use of KConfigWatcher without DBus support. You will not receive updates"
89​#endif
90​}
91​
92​void KConfigWatcher::onConfigChangeNotification(const QHash<QString, QByteArrayList> &changes)
93​{
94​ //should we ever need it we can determine the file changed with QDbusContext::message().path(), but it doesn't seem too useful
95​
96​ d->m_config->reparseConfiguration();
97​
98​ for(auto it = changes.constBegin(); it != changes.constEnd(); it++) {
99​ KConfigGroup group = d->m_config->group(QString());//top level group
100​ const auto parts = it.key().split(QLatin1Char('\x1d')); //magic char, see KConfig
101​ for(const QString &groupName: parts) {
102​ group = group.group(groupName);
103​ }
104​ emit configChanged(group, it.value());
105​ }
106​}
107​