Messages in this thread Patch in this message | | | From | Namhyung Kim <> | Date | Mon, 26 Jun 2023 13:58:48 -0700 | Subject | Re: [PATCH] perf tools: Add missing else to cmd_daemon subcommand condition |
| |
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
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 >
| |