Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221123180208.2068936-1-namhyung@kernel.org> <20221123180208.2068936-6-namhyung@kernel.org>
In-Reply-To: <20221123180208.2068936-6-namhyung@kernel.org>
From:   Ian Rogers <irogers@google.com>
Date:   Wed, 23 Nov 2022 15:23:13 -0800
Message-ID: <CAP-5=fWZ27QW+XvZx0bzEMmUbyKpmmmZudB9u=SyxiLcH3CCqw@mail.gmail.com>
Subject: Re: [PATCH 05/15] perf stat: Remove prefix argument in print_metric_headers()
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        linux-perf-users@vger.kernel.org,
        Kan Liang <kan.liang@linux.intel.com>,
        Zhengjun Xing <zhengjun.xing@linux.intel.com>,
        James Clark <james.clark@arm.com>,
        Athira Jajeev <atrajeev@linux.vnet.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It always passes a whitespace to the function, thus we can just add it to the
> function body.  Furthermore, it's only used in the normal output mode.
>
> Well, actually CSV used it but it doesn't need to since we don't care about the
> indentation or alignment in the CSV output.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/stat-display.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 744b7a40f59a..deed6ccf072f 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -996,10 +996,9 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
>  }
>
>  static void print_metric_headers_std(struct perf_stat_config *config,
> -                                    const char *prefix, bool no_indent)
> +                                    bool no_indent)
>  {
> -       if (prefix)
> -               fprintf(config->output, "%s", prefix);
> +       fputc(' ', config->output);
>
>         if (!no_indent) {
>                 int len = aggr_header_lens[config->aggr_mode];
> @@ -1012,11 +1011,8 @@ static void print_metric_headers_std(struct perf_stat_config *config,
>  }
>
>  static void print_metric_headers_csv(struct perf_stat_config *config,
> -                                    const char *prefix,
>                                      bool no_indent __maybe_unused)
>  {
> -       if (prefix)
> -               fprintf(config->output, "%s", prefix);
>         if (config->interval)
>                 fputs("time,", config->output);
>         if (!config->iostat_run)
> @@ -1024,7 +1020,6 @@ static void print_metric_headers_csv(struct perf_stat_config *config,
>  }
>
>  static void print_metric_headers_json(struct perf_stat_config *config,
> -                                     const char *prefix __maybe_unused,
>                                       bool no_indent __maybe_unused)
>  {
>         if (config->interval)
> @@ -1032,8 +1027,7 @@ static void print_metric_headers_json(struct perf_stat_config *config,
>  }
>
>  static void print_metric_headers(struct perf_stat_config *config,
> -                                struct evlist *evlist,
> -                                const char *prefix, bool no_indent)
> +                                struct evlist *evlist, bool no_indent)
>  {
>         struct evsel *counter;
>         struct outstate os = {
> @@ -1047,11 +1041,11 @@ static void print_metric_headers(struct perf_stat_config *config,
>         };
>
>         if (config->json_output)
> -               print_metric_headers_json(config, prefix, no_indent);
> +               print_metric_headers_json(config, no_indent);
>         else if (config->csv_output)
> -               print_metric_headers_csv(config, prefix, no_indent);
> +               print_metric_headers_csv(config, no_indent);
>         else
> -               print_metric_headers_std(config, prefix, no_indent);
> +               print_metric_headers_std(config, no_indent);
>
>         if (config->iostat_run)
>                 iostat_print_header_prefix(config);
> @@ -1132,7 +1126,7 @@ static void print_header_interval_std(struct perf_stat_config *config,
>         }
>
>         if (config->metric_only)
> -               print_metric_headers(config, evlist, " ", true);
> +               print_metric_headers(config, evlist, true);
>         else
>                 fprintf(output, " %*s %*s events\n",
>                         COUNTS_LEN, "counts", config->unit_width, "unit");
> @@ -1168,7 +1162,7 @@ static void print_header_std(struct perf_stat_config *config,
>         fprintf(output, ":\n\n");
>
>         if (config->metric_only)
> -               print_metric_headers(config, evlist, " ", false);
> +               print_metric_headers(config, evlist, false);
>  }
>
>  static void print_header_csv(struct perf_stat_config *config,
> @@ -1178,7 +1172,7 @@ static void print_header_csv(struct perf_stat_config *config,
>                              const char **argv __maybe_unused)
>  {
>         if (config->metric_only)
> -               print_metric_headers(config, evlist, " ", true);
> +               print_metric_headers(config, evlist, true);
>  }
>  static void print_header_json(struct perf_stat_config *config,
>                               struct target *_target __maybe_unused,
> @@ -1187,7 +1181,7 @@ static void print_header_json(struct perf_stat_config *config,
>                               const char **argv __maybe_unused)
>  {
>         if (config->metric_only)
> -               print_metric_headers(config, evlist, " ", true);
> +               print_metric_headers(config, evlist, true);
>  }
>
>  static void print_header(struct perf_stat_config *config,
> --
> 2.38.1.584.g0f3c55d4c2-goog
>