linux-kernel - Re: [PATCH v4 4/9] perf annotate: Simplify width calculation in annotation_line_

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fUM=1paD_3morMnz_6m8WWE8pmN0sC27xPanbp3FWYtjQ@mail.gmail.com>
Date: Fri, 25 Jul 2025 17:30:04 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>, 
	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 v4 4/9] perf annotate: Simplify width calculation in annotation_line__write()

On Fri, Jul 25, 2025 at 12:38 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> The width is updated after each part is printed.  It can skip the output
> processing if the total printed size is bigger than the width.
>
> No function changes intended.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/annotate.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index c21152710148b68c..d69e406c1bc289cd 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1993,6 +1993,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>                                            symbol_conf.show_nr_samples ? "Samples" : "Percent");
>                 }
>         }
> +       width -= pcnt_width;
>
>         if (notes->branch) {
>                 if (al->cycles && al->cycles->ipc)
> @@ -2056,11 +2057,12 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>                         obj__printf(obj, "%*s", ANNOTATION__AVG_IPC_WIDTH, bf);
>                 }
>         }
> +       width -= cycles_width;
>
>         obj__printf(obj, " ");
>
>         if (!*al->line)
> -               obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
> +               obj__printf(obj, "%-*s", width, " ");
>         else if (al->offset == -1) {
>                 if (al->line_nr && annotate_opts.show_linenr)
>                         printed = scnprintf(bf, sizeof(bf), "%-*d ",
> @@ -2069,7 +2071,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>                         printed = scnprintf(bf, sizeof(bf), "%-*s  ",
>                                             notes->src->widths.addr, " ");
>                 obj__printf(obj, bf);
> -               obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
> +               obj__printf(obj, "%-*s", width - printed + 1, al->line);

This doesn't seem to line up with the commit message, should width be
updated prior to this call?

>         } else {
>                 u64 addr = al->offset;
>                 int color = -1;
> @@ -2112,9 +2114,11 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>                 if (change_color)
>                         obj__set_color(obj, color);
>
> +               width -= printed + 3;

nit: For constants that like '+3' here and '+1' it'd be nice for a
comment just to say where the adjustment comes from.

Thanks,
Ian

> +
>                 disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
>
> -               obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
> +               obj__printf(obj, "%-*s", width, bf);
>
>                 (void)apd;
>         }
> --
> 2.50.1
>