Messages in this thread | | | From | Jiri Olsa <> | Date | Tue, 27 Jun 2023 10:35:28 +0200 | Subject | Re: [PATCH] perf tools: Add missing else to cmd_daemon subcommand condition |
| |
On Mon, Jun 26, 2023 at 01:58:48PM -0700, Namhyung Kim wrote: > Hi Jiri, > > On Mon, Jun 26, 2023 at 1:16 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Namhyung reported segfault in perf daemon start command. > > > > It's caused by extra check on argv[0] which is set to NULL by previous > > __cmd_start call. Adding missing else to skip the extra check. > > > > Fixes: 92294b906e6c ("perf daemon: Dynamically allocate path to perf") > > Reported-by: Namhyung Kim <namhyung@kernel.org> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > Thanks for the fix. Now it runs ok. > > Before: > $ sudo ./perf test -v daemon > 85: daemon operations : > --- start --- > test child forked, pid 82420 > test daemon list > ./tests/shell/daemon.sh: line 133: 82426 Segmentation fault > perf daemon start --config ${config} > test daemon reconfig > ./tests/shell/daemon.sh: line 133: 82520 Segmentation fault > perf daemon start --config ${config} > test daemon stop > ./tests/shell/daemon.sh: line 133: 82636 Segmentation fault > perf daemon start --config ${config} > test daemon signal > ./tests/shell/daemon.sh: line 133: 82674 Segmentation fault > perf daemon start --config ${config} > signal 12 sent to session 'test [82676]' > signal 12 sent to session 'test [82676]' > test daemon ping > ./tests/shell/daemon.sh: line 133: 82702 Segmentation fault > perf daemon start --config ${config} > test daemon lock > ./tests/shell/daemon.sh: line 133: 82734 Segmentation fault > perf daemon start --config ${config} > test child finished with 0 > ---- end ---- > daemon operations: Ok > > Maybe we need to investigate more why it was ok.. > But at least I don't see segfaults anymore
yea, for some reason parse_options would put NULL into argv[0]
I'll try to check what changed, in any case the fix makes the condition alligned with the other legs and fixes the segfault
jirka
> > > After: > 85: daemon operations : > --- start --- > test child forked, pid 80752 > test daemon list > test daemon reconfig > test daemon stop > test daemon signal > signal 12 sent to session 'test [81022]' > signal 12 sent to session 'test [81022]' > test daemon ping > test daemon lock > test child finished with 0 > ---- end ---- > daemon operations: Ok > > > Tested-by: Namhyung Kim <namhyung@kernel.org> > > Thanks, > Namhyung > > > > > --- > > tools/perf/builtin-daemon.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c > > index f5674d824a40..83954af36753 100644 > > --- a/tools/perf/builtin-daemon.c > > +++ b/tools/perf/builtin-daemon.c > > @@ -1524,7 +1524,7 @@ int cmd_daemon(int argc, const char **argv) > > if (argc) { > > if (!strcmp(argv[0], "start")) > > ret = __cmd_start(&__daemon, daemon_options, argc, argv); > > - if (!strcmp(argv[0], "signal")) > > + else if (!strcmp(argv[0], "signal")) > > ret = __cmd_signal(&__daemon, daemon_options, argc, argv); > > else if (!strcmp(argv[0], "stop")) > > ret = __cmd_stop(&__daemon, daemon_options, argc, argv); > > -- > > 2.41.0 > >
| |