Re: [PATCH v2 4/8] kernel-shark: Add logic for the plugins search path

[Slavomir Kaslev <slavomir.kaslev@gmail.com>]

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