| Message ID | 20190418152146.27232-5-ykaradzhov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Various modifications and fixes toward KS 1.0 | expand |
On Thu, Apr 18, 2019 at 6:22 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > > If the application has been started from the source code directory, all > build-in plugins will be loaded from kernel-shark/lib/ > If the application has been started from its installation location, all > build-in plugins will be loaded from INSTALL_PREFIX/lib/kshark/plugins/ > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > --- > kernel-shark/build/deff.h.cmake | 3 +++ > kernel-shark/src/KsUtils.cpp | 35 +++++++++++++++++-------- > kernel-shark/src/KsUtils.hpp | 1 + > kernel-shark/src/plugins/CMakeLists.txt | 2 +- > 4 files changed, 29 insertions(+), 12 deletions(-) > > diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake > index 8041cfc..ba211f4 100644 > --- a/kernel-shark/build/deff.h.cmake > +++ b/kernel-shark/build/deff.h.cmake > @@ -14,6 +14,9 @@ > /** KernelShark source code path. */ > #cmakedefine KS_DIR "@KS_DIR@" > > +/** KernelShark installation prefix path. */ > +#cmakedefine _INSTALL_PREFIX "@_INSTALL_PREFIX@" > + > /** Location of the trace-cmd executable. */ > #cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@" > > diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp > index 6af0c66..b05c0dc 100644 > --- a/kernel-shark/src/KsUtils.cpp > +++ b/kernel-shark/src/KsUtils.cpp > @@ -423,13 +423,12 @@ void KsPluginManager::_parsePluginList() > */ > void KsPluginManager::registerFromList(kshark_context *kshark_ctx) > { > - auto lamRegBuiltIn = [&kshark_ctx](const QString &plugin) > + auto lamRegBuiltIn = [&kshark_ctx, this](const QString &plugin) Prefixing lambda function names with 'lam' is an interesting modern form of Hungarian notation[1]. It's not useful for the reader or the compiler and it doesn't make a great style. No need to change this patch but having a clean up patch that does a bulk rename would be nice. Other than that the patch looks good to me and also for patches 2, 4, 5 and 6 Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com> Cheers, -- Slavi [1]https://en.wikipedia.org/wiki/Hungarian_notation
On Fri, 19 Apr 2019 10:43:17 +0300 Slavomir Kaslev <slavomir.kaslev@gmail.com> wrote: > > diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp > > index 6af0c66..b05c0dc 100644 > > --- a/kernel-shark/src/KsUtils.cpp > > +++ b/kernel-shark/src/KsUtils.cpp > > @@ -423,13 +423,12 @@ void KsPluginManager::_parsePluginList() > > */ > > void KsPluginManager::registerFromList(kshark_context *kshark_ctx) > > { > > - auto lamRegBuiltIn = [&kshark_ctx](const QString &plugin) > > + auto lamRegBuiltIn = [&kshark_ctx, this](const QString &plugin) > > Prefixing lambda function names with 'lam' is an interesting modern > form of Hungarian notation[1]. It's not useful for the reader or the > compiler and it doesn't make a great style. No need to change this > patch but having a clean up patch that does a bulk rename would be > nice. I don't know. Having them labeled does make it easy to know that they are lambda functions when used later. -- Steve
diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake index 8041cfc..ba211f4 100644 --- a/kernel-shark/build/deff.h.cmake +++ b/kernel-shark/build/deff.h.cmake @@ -14,6 +14,9 @@ /** KernelShark source code path. */ #cmakedefine KS_DIR "@KS_DIR@" +/** KernelShark installation prefix path. */ +#cmakedefine _INSTALL_PREFIX "@_INSTALL_PREFIX@" + /** Location of the trace-cmd executable. */ #cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@" diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp index 6af0c66..b05c0dc 100644 --- a/kernel-shark/src/KsUtils.cpp +++ b/kernel-shark/src/KsUtils.cpp @@ -423,13 +423,12 @@ void KsPluginManager::_parsePluginList() */ void KsPluginManager::registerFromList(kshark_context *kshark_ctx) { - auto lamRegBuiltIn = [&kshark_ctx](const QString &plugin) + auto lamRegBuiltIn = [&kshark_ctx, this](const QString &plugin) { char *lib; int n; - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n <= 0) return; @@ -458,13 +457,12 @@ void KsPluginManager::registerFromList(kshark_context *kshark_ctx) */ void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx) { - auto lamUregBuiltIn = [&kshark_ctx](const QString &plugin) + auto lamUregBuiltIn = [&kshark_ctx, this](const QString &plugin) { char *lib; int n; - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n <= 0) return; @@ -487,6 +485,23 @@ void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx) lamUregUser); } +char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n) +{ + QString path = QCoreApplication::applicationFilePath(); + std::string pluginStr = plugin.toStdString(); + char *lib; + + if (path.contains(KS_DIR)) { + n = asprintf(&lib, "%s/lib/plugin-%s.so", + KS_DIR, pluginStr.c_str()); + } else { + n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so", + _INSTALL_PREFIX, pluginStr.c_str()); + } + + return lib; +} + /** * @brief Register a Plugin. * @@ -508,8 +523,7 @@ void KsPluginManager::registerPlugin(const QString &plugin) * The argument is the name of the plugin. From the * name get the library .so file. */ - n = asprintf(&lib, "%s/lib/plugin-%s.so", - KS_DIR, plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n > 0) { kshark_register_plugin(kshark_ctx, lib); _registeredKsPlugins[i] = true; @@ -570,8 +584,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin) * The argument is the name of the plugin. From the * name get the library .so file. */ - n = asprintf(&lib, "%s/lib/plugin-%s.so", KS_DIR, - plugin.toStdString().c_str()); + lib = _pluginLibFromName(plugin, n); if (n > 0) { kshark_unregister_plugin(kshark_ctx, lib); _registeredKsPlugins[i] = false; @@ -579,7 +592,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin) } return; - } else if (plugin.contains("/lib/plugin-" + + } else if (plugin.contains("/lib/plugin-" + _ksPluginList[i], Qt::CaseInsensitive)) { /* * The argument is the name of the library .so file. diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp index 877c62a..85da9ae 100644 --- a/kernel-shark/src/KsUtils.hpp +++ b/kernel-shark/src/KsUtils.hpp @@ -238,6 +238,7 @@ signals: private: void _parsePluginList(); + char *_pluginLibFromName(const QString &plugin, int &n); template <class T> void _forEachInList(const QStringList &pl, diff --git a/kernel-shark/src/plugins/CMakeLists.txt b/kernel-shark/src/plugins/CMakeLists.txt index 6098275..64cf98d 100644 --- a/kernel-shark/src/plugins/CMakeLists.txt +++ b/kernel-shark/src/plugins/CMakeLists.txt @@ -29,6 +29,6 @@ BUILD_PLUGIN(NAME missed_events list(APPEND PLUGIN_LIST "missed_events default") # This plugin will be loaded by default install(TARGETS sched_events missed_events - LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/) + LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/plugins/) set(PLUGINS ${PLUGIN_LIST} PARENT_SCOPE)
If the application has been started from the source code directory, all build-in plugins will be loaded from kernel-shark/lib/ If the application has been started from its installation location, all build-in plugins will be loaded from INSTALL_PREFIX/lib/kshark/plugins/ Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> --- kernel-shark/build/deff.h.cmake | 3 +++ kernel-shark/src/KsUtils.cpp | 35 +++++++++++++++++-------- kernel-shark/src/KsUtils.hpp | 1 + kernel-shark/src/plugins/CMakeLists.txt | 2 +- 4 files changed, 29 insertions(+), 12 deletions(-)