Skip to content

Commit

Permalink
refacotr: Using raw pointers instead of smart pointers in extension m…
Browse files Browse the repository at this point in the history
…odules

External plugins may manage memory on their own, which may result in double free here

Log: fix coredump
  • Loading branch information
Johnson-zs committed Aug 19, 2024
1 parent df2be8e commit 218745d
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void EmblemIconWorker::onFetchEmblemIcons(const QList<QPair<QString, int>> &loca
return;

const auto &emblemPlugins = ExtensionPluginManager::instance().emblemPlugins();
std::for_each(emblemPlugins.begin(), emblemPlugins.end(), [&localPaths, this](QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin> plugin) {
std::for_each(emblemPlugins.begin(), emblemPlugins.end(), [&localPaths, this](DFMEXT::DFMExtEmblemIconPlugin *plugin) {
Q_ASSERT(plugin);
for (const auto &path : localPaths) {
if (this->parseLocationEmblemIcons(path.first, path.second, plugin))
Expand All @@ -74,13 +74,13 @@ void EmblemIconWorker::onClearCache()
pluginCaches.clear();
}

bool EmblemIconWorker::parseLocationEmblemIcons(const QString &path, int count, QSharedPointer<dfmext::DFMExtEmblemIconPlugin> plugin)
bool EmblemIconWorker::parseLocationEmblemIcons(const QString &path, int count, dfmext::DFMExtEmblemIconPlugin *plugin)
{
const auto &emblem { plugin->locationEmblemIcons(path.toStdString(), count) };
const std::vector<DFMEXT::DFMExtEmblemIconLayout> &layouts { emblem.emblems() };
// why add `pluginCaches` ?
// To clear the emblem icon when a plugin returns an empty `DFMExtEmblemIconLayout`.
quint64 pluginAddr { reinterpret_cast<quint64>(plugin.data()) };
quint64 pluginAddr { reinterpret_cast<quint64>(plugin) };
const CacheType &curPluginCache { pluginCaches.value(pluginAddr) };
if (layouts.empty() && curPluginCache.value(path).isEmpty())
return false;
Expand Down Expand Up @@ -108,9 +108,9 @@ bool EmblemIconWorker::parseLocationEmblemIcons(const QString &path, int count,
return true;
}

void EmblemIconWorker::parseEmblemIcons(const QString &path, int count, QSharedPointer<dfmext::DFMExtEmblemIconPlugin> plugin)
void EmblemIconWorker::parseEmblemIcons(const QString &path, int count, dfmext::DFMExtEmblemIconPlugin *plugin)
{
quint64 pluginAddr { reinterpret_cast<quint64>(plugin.data()) };
quint64 pluginAddr { reinterpret_cast<quint64>(plugin) };
if (hasCachedByOtherLocationEmblem(path, pluginAddr))
return;
const std::vector<std::string> &icons { plugin->emblemIcons(path.toStdString()) };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ public Q_SLOTS:

private:
// method 2
bool parseLocationEmblemIcons(const QString &path, int count, QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin> plugin);
bool parseLocationEmblemIcons(const QString &path, int count, DFMEXT::DFMExtEmblemIconPlugin *plugin);
// method 1
void parseEmblemIcons(const QString &path, int count, QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin> plugin);
void parseEmblemIcons(const QString &path, int count, DFMEXT::DFMExtEmblemIconPlugin *plugin);

CacheType makeCache(const QString &path, const QList<QPair<QString, int>> &group);
void makeLayoutGroup(const std::vector<DFMEXT::DFMExtEmblemIconLayout> &layouts, QList<QPair<QString, int>> *group);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,15 @@ void ExtensionPluginManagerPrivate::doAppendExt(const QString &name, ExtPluginLo

DFMEXT::DFMExtMenuPlugin *menu { loader->resolveMenuPlugin() };
if (menu)
menuMap.insert(name, QSharedPointer<DFMEXT::DFMExtMenuPlugin>(menu));
menuMap.insert(name, menu);

DFMEXT::DFMExtEmblemIconPlugin *emblem { loader->resolveEmblemPlugin() };
if (emblem)
emblemMap.insert(name, QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin>(emblem));
emblemMap.insert(name, emblem);

DFMEXT::DFMExtWindowPlugin *window { loader->resolveWindowPlugin() };
if (window)
windowMap.insert(name, QSharedPointer<DFMEXT::DFMExtWindowPlugin>(window));
windowMap.insert(name, window);
}

void ExtensionPluginManagerPrivate::release()
Expand Down Expand Up @@ -237,21 +237,21 @@ bool ExtensionPluginManager::exists(ExtensionPluginManager::ExtensionType type)
return false;
}

QList<QSharedPointer<dfmext::DFMExtMenuPlugin>> ExtensionPluginManager::menuPlugins() const
QList<DFMEXT::DFMExtMenuPlugin *> ExtensionPluginManager::menuPlugins() const
{
Q_D(const ExtensionPluginManager);

return d->menuMap.values();
}

QList<QSharedPointer<dfmext::DFMExtEmblemIconPlugin>> ExtensionPluginManager::emblemPlugins() const
QList<DFMEXT::DFMExtEmblemIconPlugin *> ExtensionPluginManager::emblemPlugins() const
{
Q_D(const ExtensionPluginManager);

return d->emblemMap.values();
}

QList<QSharedPointer<dfmext::DFMExtWindowPlugin>> ExtensionPluginManager::windowPlugins() const
QList<DFMEXT::DFMExtWindowPlugin *> ExtensionPluginManager::windowPlugins() const
{
Q_D(const ExtensionPluginManager);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ class ExtensionPluginManager : public QObject
InitState currentState() const;
bool initialized() const;
bool exists(ExtensionType type) const;
QList<QSharedPointer<DFMEXT::DFMExtMenuPlugin>> menuPlugins() const;
QList<QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin>> emblemPlugins() const;
QList<QSharedPointer<DFMEXT::DFMExtWindowPlugin>> windowPlugins() const;
QList<DFMEXT::DFMExtMenuPlugin *> menuPlugins() const;
QList<DFMEXT::DFMExtEmblemIconPlugin *> emblemPlugins() const;
QList<DFMEXT::DFMExtWindowPlugin *> windowPlugins() const;

DFMEXT::DFMExtMenuProxy *pluginMenuProxy() const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class ExtensionPluginManagerPrivate : public QObject
Q_DECLARE_PUBLIC(ExtensionPluginManager)

public:
using DFMExtMenuPluginMap = QMap<QString, QSharedPointer<DFMEXT::DFMExtMenuPlugin>>;
using DFMExtEmblemPluginMap = QMap<QString, QSharedPointer<DFMEXT::DFMExtEmblemIconPlugin>>;
using DFMExtWindowPluginMap = QMap<QString, QSharedPointer<DFMEXT::DFMExtWindowPlugin>>;
using DFMExtMenuPluginMap = QMap<QString, DFMEXT::DFMExtMenuPlugin *>;
using DFMExtEmblemPluginMap = QMap<QString, DFMEXT::DFMExtEmblemIconPlugin *>;
using DFMExtWindowPluginMap = QMap<QString, DFMEXT::DFMExtWindowPlugin *>;

explicit ExtensionPluginManagerPrivate(ExtensionPluginManager *qq);
~ExtensionPluginManagerPrivate() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ static void doActionForEveryPlugin(std::function<void(DFMEXT::DFMExtWindowPlugin
return;
}
const auto &windowPlugins { ExtensionPluginManager::instance().windowPlugins() };
std::for_each(windowPlugins.begin(), windowPlugins.end(), [callback](QSharedPointer<DFMEXT::DFMExtWindowPlugin> plugin) {
std::for_each(windowPlugins.begin(), windowPlugins.end(), [callback](DFMEXT::DFMExtWindowPlugin *plugin) {
Q_ASSERT(plugin);
callback(plugin.data());
callback(plugin);
});
}

Expand Down

0 comments on commit 218745d

Please sign in to comment.