| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fWSg07fTLJNya0ftdUfk7HTbYXB=5xbbPBeN_q4LNs6cw@mail.gmail.com>
Date: Wed, 19 Nov 2025 21:30:25 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, James Clark <james.clark@...aro.org>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v2 2/3] perf list: Share print state with JSON output
On Wed, Nov 19, 2025 at 4:47 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> The JSON print state has only one different field (need_sep). Let's
> add the default print state to the json state and use it. Then we can
> use the 'ps' variable to update the state properly.
>
> This is a preparation for the next commit.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
Reviewed-by: Ian Rogers <irogers@...gle.com>
Thanks,
Ian
> ---
> tools/perf/builtin-list.c | 127 +++++++++++++++++++-------------------
> 1 file changed, 65 insertions(+), 62 deletions(-)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 28bf1fc7f5eeff8f..6c5913f129f39c94 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -283,8 +283,8 @@ static void default_print_metric(void *ps,
> }
>
> struct json_print_state {
> - /** @fp: File to write output to. */
> - FILE *fp;
> + /** The shared print_state */
> + struct print_state common;
> /** Should a separator be printed prior to the next item? */
> bool need_sep;
> };
> @@ -292,7 +292,7 @@ struct json_print_state {
> static void json_print_start(void *ps)
> {
> struct json_print_state *print_state = ps;
> - FILE *fp = print_state->fp;
> + FILE *fp = print_state->common.fp;
>
> fprintf(fp, "[\n");
> }
> @@ -300,7 +300,7 @@ static void json_print_start(void *ps)
> static void json_print_end(void *ps)
> {
> struct json_print_state *print_state = ps;
> - FILE *fp = print_state->fp;
> + FILE *fp = print_state->common.fp;
>
> fprintf(fp, "%s]\n", print_state->need_sep ? "\n" : "");
> }
> @@ -370,7 +370,7 @@ static void json_print_event(void *ps, const char *topic,
> {
> struct json_print_state *print_state = ps;
> bool need_sep = false;
> - FILE *fp = print_state->fp;
> + FILE *fp = print_state->common.fp;
> struct strbuf buf;
>
> strbuf_init(&buf, 0);
> @@ -446,7 +446,7 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> {
> struct json_print_state *print_state = ps;
> bool need_sep = false;
> - FILE *fp = print_state->fp;
> + FILE *fp = print_state->common.fp;
> struct strbuf buf;
>
> strbuf_init(&buf, 0);
> @@ -521,10 +521,12 @@ int cmd_list(int argc, const char **argv)
> .fp = stdout,
> .desc = true,
> };
> - struct print_state json_ps = {
> - .fp = stdout,
> + struct json_print_state json_ps = {
> + .common = {
> + .fp = stdout,
> + },
> };
> - void *ps = &default_ps;
> + struct print_state *ps = &default_ps;
> struct print_callbacks print_cb = {
> .print_start = default_print_start,
> .print_end = default_print_end,
> @@ -572,9 +574,11 @@ int cmd_list(int argc, const char **argv)
> argc = parse_options(argc, argv, list_options, list_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
>
> + if (json)
> + ps = &json_ps.common;
> +
> if (output_path) {
> - default_ps.fp = fopen(output_path, "w");
> - json_ps.fp = default_ps.fp;
> + ps->fp = fopen(output_path, "w");
> }
>
> setup_pager();
> @@ -590,14 +594,13 @@ int cmd_list(int argc, const char **argv)
> .print_metric = json_print_metric,
> .skip_duplicate_pmus = json_skip_duplicate_pmus,
> };
> - ps = &json_ps;
> } else {
> - default_ps.last_topic = strdup("");
> - assert(default_ps.last_topic);
> - default_ps.visited_metrics = strlist__new(NULL, NULL);
> - assert(default_ps.visited_metrics);
> + ps->last_topic = strdup("");
> + assert(ps->last_topic);
> + ps->visited_metrics = strlist__new(NULL, NULL);
> + assert(ps->visited_metrics);
> if (unit_name)
> - default_ps.pmu_glob = strdup(unit_name);
> + ps->pmu_glob = strdup(unit_name);
> else if (cputype) {
> const struct perf_pmu *pmu = perf_pmus__pmu_for_pmu_filter(cputype);
>
> @@ -606,15 +609,15 @@ int cmd_list(int argc, const char **argv)
> ret = -1;
> goto out;
> }
> - default_ps.pmu_glob = strdup(pmu->name);
> + ps->pmu_glob = strdup(pmu->name);
> }
> }
> print_cb.print_start(ps);
>
> if (argc == 0) {
> if (!unit_name) {
> - default_ps.metrics = true;
> - default_ps.metricgroups = true;
> + ps->metrics = true;
> + ps->metricgroups = true;
> }
> print_events(&print_cb, ps);
> goto out;
> @@ -636,57 +639,57 @@ int cmd_list(int argc, const char **argv)
> default_ps.pmu_glob = old_pmu_glob;
> } else if (strcmp(argv[i], "hw") == 0 ||
> strcmp(argv[i], "hardware") == 0) {
> - char *old_event_glob = default_ps.event_glob;
> + char *old_event_glob = ps->event_glob;
>
> - default_ps.event_glob = strdup("legacy hardware");
> - if (!default_ps.event_glob) {
> + ps->event_glob = strdup("legacy hardware");
> + if (!ps->event_glob) {
> ret = -1;
> goto out;
> }
> perf_pmus__print_pmu_events(&print_cb, ps);
> - zfree(&default_ps.event_glob);
> - default_ps.event_glob = old_event_glob;
> + zfree(&ps->event_glob);
> + ps->event_glob = old_event_glob;
> } else if (strcmp(argv[i], "sw") == 0 ||
> strcmp(argv[i], "software") == 0) {
> - char *old_pmu_glob = default_ps.pmu_glob;
> + char *old_pmu_glob = ps->pmu_glob;
> static const char * const sw_globs[] = { "software", "tool" };
>
> for (size_t j = 0; j < ARRAY_SIZE(sw_globs); j++) {
> - default_ps.pmu_glob = strdup(sw_globs[j]);
> - if (!default_ps.pmu_glob) {
> + ps->pmu_glob = strdup(sw_globs[j]);
> + if (!ps->pmu_glob) {
> ret = -1;
> goto out;
> }
> perf_pmus__print_pmu_events(&print_cb, ps);
> - zfree(&default_ps.pmu_glob);
> + zfree(&ps->pmu_glob);
> }
> - default_ps.pmu_glob = old_pmu_glob;
> + ps->pmu_glob = old_pmu_glob;
> } else if (strcmp(argv[i], "cache") == 0 ||
> strcmp(argv[i], "hwcache") == 0) {
> - char *old_event_glob = default_ps.event_glob;
> + char *old_event_glob = ps->event_glob;
>
> - default_ps.event_glob = strdup("legacy cache");
> - if (!default_ps.event_glob) {
> + ps->event_glob = strdup("legacy cache");
> + if (!ps->event_glob) {
> ret = -1;
> goto out;
> }
> perf_pmus__print_pmu_events(&print_cb, ps);
> - zfree(&default_ps.event_glob);
> - default_ps.event_glob = old_event_glob;
> + zfree(&ps->event_glob);
> + ps->event_glob = old_event_glob;
> } else if (strcmp(argv[i], "pmu") == 0) {
> - default_ps.exclude_abi = true;
> + ps->exclude_abi = true;
> perf_pmus__print_pmu_events(&print_cb, ps);
> - default_ps.exclude_abi = false;
> + ps->exclude_abi = false;
> } else if (strcmp(argv[i], "sdt") == 0)
> print_sdt_events(&print_cb, ps);
> else if (strcmp(argv[i], "metric") == 0 || strcmp(argv[i], "metrics") == 0) {
> - default_ps.metricgroups = false;
> - default_ps.metrics = true;
> + ps->metricgroups = false;
> + ps->metrics = true;
> metricgroup__print(&print_cb, ps);
> } else if (strcmp(argv[i], "metricgroup") == 0 ||
> strcmp(argv[i], "metricgroups") == 0) {
> - default_ps.metricgroups = true;
> - default_ps.metrics = false;
> + ps->metricgroups = true;
> + ps->metrics = false;
> metricgroup__print(&print_cb, ps);
> }
> #ifdef HAVE_LIBPFM
> @@ -694,40 +697,40 @@ int cmd_list(int argc, const char **argv)
> print_libpfm_events(&print_cb, ps);
> #endif
> else if ((sep = strchr(argv[i], ':')) != NULL) {
> - char *old_pmu_glob = default_ps.pmu_glob;
> - char *old_event_glob = default_ps.event_glob;
> + char *old_pmu_glob = ps->pmu_glob;
> + char *old_event_glob = ps->event_glob;
>
> - default_ps.event_glob = strdup(argv[i]);
> - if (!default_ps.event_glob) {
> + ps->event_glob = strdup(argv[i]);
> + if (!ps->event_glob) {
> ret = -1;
> goto out;
> }
>
> - default_ps.pmu_glob = strdup("tracepoint");
> - if (!default_ps.pmu_glob) {
> - zfree(&default_ps.event_glob);
> + ps->pmu_glob = strdup("tracepoint");
> + if (!ps->pmu_glob) {
> + zfree(&ps->event_glob);
> ret = -1;
> goto out;
> }
> perf_pmus__print_pmu_events(&print_cb, ps);
> - zfree(&default_ps.pmu_glob);
> - default_ps.pmu_glob = old_pmu_glob;
> + zfree(&ps->pmu_glob);
> + ps->pmu_glob = old_pmu_glob;
> print_sdt_events(&print_cb, ps);
> - default_ps.metrics = true;
> - default_ps.metricgroups = true;
> + ps->metrics = true;
> + ps->metricgroups = true;
> metricgroup__print(&print_cb, ps);
> - zfree(&default_ps.event_glob);
> - default_ps.event_glob = old_event_glob;
> + zfree(&ps->event_glob);
> + ps->event_glob = old_event_glob;
> } else {
> if (asprintf(&s, "*%s*", argv[i]) < 0) {
> printf("Critical: Not enough memory! Trying to continue...\n");
> continue;
> }
> - default_ps.event_glob = s;
> + ps->event_glob = s;
> perf_pmus__print_pmu_events(&print_cb, ps);
> print_sdt_events(&print_cb, ps);
> - default_ps.metrics = true;
> - default_ps.metricgroups = true;
> + ps->metrics = true;
> + ps->metricgroups = true;
> metricgroup__print(&print_cb, ps);
> free(s);
> }
> @@ -735,12 +738,12 @@ int cmd_list(int argc, const char **argv)
>
> out:
> print_cb.print_end(ps);
> - free(default_ps.pmu_glob);
> - free(default_ps.last_topic);
> - free(default_ps.last_metricgroups);
> - strlist__delete(default_ps.visited_metrics);
> + free(ps->pmu_glob);
> + free(ps->last_topic);
> + free(ps->last_metricgroups);
> + strlist__delete(ps->visited_metrics);
> if (output_path)
> - fclose(default_ps.fp);
> + fclose(ps->fp);
>
> return ret;
> }
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
Powered by blists - more mailing lists