On Tue, Apr 9, 2019 at 6:11 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Tue, 9 Apr 2019 14:59:54 +0000
> Slavomir Kaslev <kaslevs@xxxxxxxxxx> wrote:
>
> > We also have the same problem with KS_DIR and TRACECMD_BIN_DIR though.
> > They both point inside the trace-cmd source directory for the user who
> > built kernelshark atm.
>
> Hmm, these are a bit different. I'm not sure I want them to be user
> run-time configurable, because they are executing system tools. That
> could be a security concern.
>
> But they should be compile time configurable, such that a distro can
> define them.
I agree, there's no need for those to have environment variable override.
The problem is the way they are defined atm:
# First search in the user provided paths.
find_path(TRACECMD_BIN_DIR NAMES trace-cmd
PATHS $ENV{TRACE_CMD}/tracecmd/
${CMAKE_SOURCE_DIR}/../tracecmd/
NO_DEFAULT_PATH)
This will never evaluate to /usr/bin which is what it should be for
distro builds.
KS_DIR is used runtime to load plugins, as default path for file open
dialogs (which doesn't make much sense imo) and at compile time to
construct relative paths in the source directory. It is currently
defined simply as
set(KS_DIR ${CMAKE_SOURCE_DIR})
I think there needs to be a separate constant for plugins path that
can be set at build time and in distro build can point to
/usr/share/kernelshark/plugins or somewhere where we want shared
plugins to live. The compile time uses of KS_DIR are fine though
${CMAKE_SOURCE_DIR} is clearer for the reader. The default directory
for file open dialogs can be the current directory or the last opened
path rather than the kernelshark source directory.
What do you think?
Cheers,
-- Slavi
|