Re: [PATCH v4 4/9] perf annotate: Simplify width calculation in annotation_line__write()

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Jul 25, 2025 at 05:30:04PM -0700, Ian Rogers wrote:
> On Fri, Jul 25, 2025 at 12:38 PM Namhyung Kim <namhyung@kernel.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@kernel.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?

Fair enough.

> 
> >         } 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.

Sure, will do.

Thanks,
Namhyung
 
> > +
> >                 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
> >