Skip to content

Commit

Permalink
libsubcmd: Don't free the usage string
Browse files Browse the repository at this point in the history
Currently, commands which depend on 'parse_options_subcommand()' don't
show the usage string, and instead show '(null)'

    $ ./perf sched
	Usage: (null)

    -D, --dump-raw-trace  dump raw trace in ASCII
    -f, --force           don't complain, do it
    -i, --input <file>    input file name
    -v, --verbose         be more verbose (show symbol address, etc)

'parse_options_subcommand()' is generally expected to initialise the usage
string, with information in the passed 'subcommands[]' array

This behaviour was changed in:

  230a7a7 ("libsubcmd: Fix parse-options memory leak")

Where the generated usage string is deallocated, and usage[0] string is
reassigned as NULL.

As discussed in [1], free the allocated usage string in the main
function itself, and don't reset usage string to NULL in
parse_options_subcommand

With this change, the behaviour is restored.

    $ ./perf sched
        Usage: perf sched [<options>] {record|latency|map|replay|script|timehist}

           -D, --dump-raw-trace  dump raw trace in ASCII
           -f, --force           don't complain, do it
           -i, --input <file>    input file name
           -v, --verbose         be more verbose (show symbol address, etc)

[1]: https://lore.kernel.org/linux-perf-users/htq5vhx6piet4nuq2mmhk7fs2bhfykv52dbppwxmo3s7du2odf@styd27tioc6e/

Fixes: 230a7a7 ("libsubcmd: Fix parse-options memory leak")
Suggested-by: Namhyung Kim <[email protected]>
Signed-off-by: Aditya Gupta <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Athira Rajeev <[email protected]>
Cc: Disha Goel <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Kajol Jain <[email protected]>
Cc: Madhavan Srinivasan <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
adi-g15-ibm authored and acmel committed Sep 4, 2024
1 parent fa6cc3f commit 1a5efc9
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 5 deletions.
8 changes: 3 additions & 5 deletions tools/lib/subcmd/parse-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,11 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
const char *const subcommands[], const char *usagestr[], int flags)
{
struct parse_opt_ctx_t ctx;
char *buf = NULL;

/* build usage string if it's not provided */
if (subcommands && !usagestr[0]) {
char *buf = NULL;

astrcatf(&buf, "%s %s [<options>] {", subcmd_config.exec_name, argv[0]);

for (int i = 0; subcommands[i]; i++) {
Expand Down Expand Up @@ -678,10 +679,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
astrcatf(&error_buf, "unknown switch `%c'", *ctx.opt);
usage_with_options(usagestr, options);
}
if (buf) {
usagestr[0] = NULL;
free(buf);
}

return parse_options_end(&ctx);
}

Expand Down
2 changes: 2 additions & 0 deletions tools/perf/builtin-kmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -2057,6 +2057,8 @@ int cmd_kmem(int argc, const char **argv)

out_delete:
perf_session__delete(session);
/* free usage string allocated by parse_options_subcommand */
free((void *)kmem_usage[0]);

return ret;
}
3 changes: 3 additions & 0 deletions tools/perf/builtin-kvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2184,5 +2184,8 @@ int cmd_kvm(int argc, const char **argv)
else
usage_with_options(kvm_usage, kvm_options);

/* free usage string allocated by parse_options_subcommand */
free((void *)kvm_usage[0]);

return 0;
}
3 changes: 3 additions & 0 deletions tools/perf/builtin-kwork.c
Original file line number Diff line number Diff line change
Expand Up @@ -2519,5 +2519,8 @@ int cmd_kwork(int argc, const char **argv)
} else
usage_with_options(kwork_usage, kwork_options);

/* free usage string allocated by parse_options_subcommand */
free((void *)kwork_usage[0]);

return 0;
}
3 changes: 3 additions & 0 deletions tools/perf/builtin-lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -2712,6 +2712,9 @@ int cmd_lock(int argc, const char **argv)
usage_with_options(lock_usage, lock_options);
}

/* free usage string allocated by parse_options_subcommand */
free((void *)lock_usage[0]);

zfree(&lockhash_table);
return rc;
}
3 changes: 3 additions & 0 deletions tools/perf/builtin-mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -546,5 +546,8 @@ int cmd_mem(int argc, const char **argv)
else
usage_with_options(mem_usage, mem_options);

/* free usage string allocated by parse_options_subcommand */
free((void *)mem_usage[0]);

return 0;
}
3 changes: 3 additions & 0 deletions tools/perf/builtin-sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -3954,5 +3954,8 @@ int cmd_sched(int argc, const char **argv)
usage_with_options(sched_usage, sched_options);
}

/* free usage string allocated by parse_options_subcommand */
free((void *)sched_usage[0]);

return 0;
}

0 comments on commit 1a5efc9

Please sign in to comment.