Re: [RFT PATCH v2 1/7] perf stat: Check color's length instead of the pointer

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@xxxxxxxxxx> wrote:
>
> Color string returned by metric_threshold_classify__color() is never
> NULL, check the presence of *color will always return true.
>
> Fix this by change the checks against length of *color.
>
> Fixes: 37b77ae95416 ("perf stat: Change color to threshold in print_metric")
> Signed-off-by: Yushan Wang <wangyushan12@xxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/perf/builtin-script.c    | 2 +-
>  tools/perf/util/stat-display.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c8ac9f01a36b..58cd5cd377a6 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2111,7 +2111,7 @@ static void script_print_metric(struct perf_stat_config *config __maybe_unused,
>         perf_sample__fprintf_start(NULL, mctx->sample, mctx->thread, mctx->evsel,
>                                    PERF_RECORD_SAMPLE, mctx->fp);
>         fputs("\tmetric: ", mctx->fp);
> -       if (color)
> +       if (strlen(color))
>                 color_fprintf(mctx->fp, color, fmt, val);
>         else
>                 printf(fmt, val);
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 993f4c4b8f44..74cba80c24b0 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -474,7 +474,7 @@ static void print_metric_std(struct perf_stat_config *config,
>                 do_new_line_std(config, os);
>
>         n = fprintf(out, " # ");
> -       if (color)
> +       if (strlen(color))
>                 n += color_fprintf(out, color, fmt, val);
>         else
>                 n += fprintf(out, fmt, val);
> @@ -607,7 +607,7 @@ static void print_metric_only(struct perf_stat_config *config,
>         if (mlen < strlen(unit))
>                 mlen = strlen(unit) + 1;
>
> -       if (color)
> +       if (strlen(color))
>                 mlen += strlen(color) + sizeof(PERF_COLOR_RESET) - 1;
>
>         color_snprintf(str, sizeof(str), color ?: "", fmt ?: "", val);
> --
> 2.33.0
>