From d5dcebed83b21928975a641834dc08875407ff4e Mon Sep 17 00:00:00 2001 From: Johannes Lorenz Date: Fri, 18 Jan 2019 23:22:52 +0100 Subject: [PATCH] Use QString for SubPluginFeatures' virtuals The former virtuals returned `const char*`, which lead to invalid reads when `LadspaSubPluginFeatures` returned pointers to temporary `QByteArray::data`. --- include/Plugin.h | 12 +++++------ .../LadspaEffect/LadspaSubPluginFeatures.cpp | 4 ++-- .../LadspaEffect/LadspaSubPluginFeatures.h | 2 +- src/core/Plugin.cpp | 20 ++++++++++++++----- src/core/PluginFactory.cpp | 8 ++++---- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/include/Plugin.h b/include/Plugin.h index 034d8a06c..48c5f90c7 100644 --- a/include/Plugin.h +++ b/include/Plugin.h @@ -165,9 +165,9 @@ public: // helper functions to retrieve data that is // not part of the key, but mapped via desc->subPluginFeatures - const char* additionalFileExtensions() const; - const char* displayName() const; - const char* description() const; + QString additionalFileExtensions() const; + QString displayName() const; + QString description() const; const PixmapLoader* logo() const; } ; @@ -200,19 +200,19 @@ public: // The defaults are sane, i.e. redirect to sub plugins // supererior descriptor - virtual const char* additionalFileExtensions(const Key&) const + virtual QString additionalFileExtensions(const Key&) const { return nullptr; } - virtual const char* displayName(const Key& k) const + virtual QString displayName(const Key& k) const { return k.isValid() ? k.desc->displayName : k.name.toUtf8().data(); } - virtual const char* description(const Key& k) const + virtual QString description(const Key& k) const { return k.isValid() ? k.desc->description : ""; } diff --git a/plugins/LadspaEffect/LadspaSubPluginFeatures.cpp b/plugins/LadspaEffect/LadspaSubPluginFeatures.cpp index 1b055fe73..4cefa90b5 100644 --- a/plugins/LadspaEffect/LadspaSubPluginFeatures.cpp +++ b/plugins/LadspaEffect/LadspaSubPluginFeatures.cpp @@ -44,11 +44,11 @@ LadspaSubPluginFeatures::LadspaSubPluginFeatures( Plugin::PluginTypes _type ) : -const char *LadspaSubPluginFeatures::displayName(const Plugin::Descriptor::SubPluginFeatures::Key &k) const +QString LadspaSubPluginFeatures::displayName(const Plugin::Descriptor::SubPluginFeatures::Key &k) const { const ladspa_key_t & lkey = subPluginKeyToLadspaKey(&k); Ladspa2LMMS * lm = Engine::getLADSPAManager(); - return lm->getName(lkey).toUtf8().data(); + return lm->getName(lkey); } diff --git a/plugins/LadspaEffect/LadspaSubPluginFeatures.h b/plugins/LadspaEffect/LadspaSubPluginFeatures.h index b7613827b..3f47734f9 100644 --- a/plugins/LadspaEffect/LadspaSubPluginFeatures.h +++ b/plugins/LadspaEffect/LadspaSubPluginFeatures.h @@ -37,7 +37,7 @@ class LadspaSubPluginFeatures : public Plugin::Descriptor::SubPluginFeatures public: LadspaSubPluginFeatures( Plugin::PluginTypes _type ); - const char* displayName(const Key& k) const override; + QString displayName(const Key& k) const override; void fillDescriptionWidget( QWidget * _parent, const Key * _key ) const override; diff --git a/src/core/Plugin.cpp b/src/core/Plugin.cpp index 7fcbd9795..2975cf104 100644 --- a/src/core/Plugin.cpp +++ b/src/core/Plugin.cpp @@ -89,6 +89,14 @@ T use_this_or(T this_param, T or_param) +QString use_this_or(QString this_param, QString or_param) +{ + return this_param.isNull() ? or_param : this_param; +} + + + + QString Plugin::displayName() const { return Model::displayName().isEmpty() // currently always empty @@ -113,7 +121,7 @@ const PixmapLoader* Plugin::logo() const -const char *Plugin::Descriptor::SubPluginFeatures::Key::additionalFileExtensions() const +QString Plugin::Descriptor::SubPluginFeatures::Key::additionalFileExtensions() const { Q_ASSERT(isValid()); return desc->subPluginFeatures @@ -126,12 +134,13 @@ const char *Plugin::Descriptor::SubPluginFeatures::Key::additionalFileExtensions -const char* Plugin::Descriptor::SubPluginFeatures::Key::displayName() const +QString Plugin::Descriptor::SubPluginFeatures::Key::displayName() const { Q_ASSERT(isValid()); return desc->subPluginFeatures // get from sub plugin - ? use_this_or(desc->subPluginFeatures->displayName(*this), desc->displayName) + ? use_this_or(desc->subPluginFeatures->displayName(*this), + QString::fromUtf8(desc->displayName)) // get from plugin : desc->displayName; } @@ -150,11 +159,12 @@ const PixmapLoader* Plugin::Descriptor::SubPluginFeatures::Key::logo() const -const char *Plugin::Descriptor::SubPluginFeatures::Key::description() const +QString Plugin::Descriptor::SubPluginFeatures::Key::description() const { Q_ASSERT(isValid()); return desc->subPluginFeatures - ? use_this_or(desc->subPluginFeatures->description(*this), desc->description) + ? use_this_or(desc->subPluginFeatures->description(*this), + QString::fromUtf8(desc->description)) : desc->description; } diff --git a/src/core/PluginFactory.cpp b/src/core/PluginFactory.cpp index ec390ae6e..f84227091 100644 --- a/src/core/PluginFactory.cpp +++ b/src/core/PluginFactory.cpp @@ -194,12 +194,12 @@ void PluginFactory::discoverPlugins() pluginInfos << info; auto addSupportedFileTypes = - [this](const char* supportedFileTypes, + [this](QString supportedFileTypes, const PluginInfo& info, const Plugin::Descriptor::SubPluginFeatures::Key* key = nullptr) { - if(supportedFileTypes) - for (const QString& ext : QString(supportedFileTypes).split(',')) + if(!supportedFileTypes.isNull()) + for (const QString& ext : supportedFileTypes.split(',')) { //qDebug() << "Plugin " << info.name() << "supports" << ext; PluginInfoAndKey infoAndKey; @@ -212,7 +212,7 @@ void PluginFactory::discoverPlugins() }; if (info.descriptor->supportedFileTypes) - addSupportedFileTypes(info.descriptor->supportedFileTypes, info); + addSupportedFileTypes(QString(info.descriptor->supportedFileTypes), info); if (info.descriptor->subPluginFeatures) {