There are three different but similar functions for annotation on TUI.
Rename it to __hist_entry__tui_annotate() and make sure it passes 'he'.
It's not used for now but it'll be needed for later use.
Also remove map_symbol__tui_annotate() which was a simple wrapper.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 17 +++++++----------
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/annotate.h | 12 ------------
tools/perf/util/hist.h | 12 +++++++-----
4 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 183902dac042ecb0..28ef146f29e8e742 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -27,6 +27,7 @@ struct annotate_browser {
struct rb_node *curr_hot;
struct annotation_line *selection;
struct arch *arch;
+ struct hist_entry *he;
bool searching_backwards;
char search_bf[128];
};
@@ -557,7 +558,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
target_ms.map = ms->map;
target_ms.sym = dl->ops.target.sym;
annotation__unlock(notes);
- symbol__tui_annotate(&target_ms, evsel, hbt);
+ __hist_entry__tui_annotate(browser->he, &target_ms, evsel, hbt);
sym_title(ms->sym, ms->map, title, sizeof(title), annotate_opts.percent_type);
ui_browser__show_title(&browser->b, title);
return true;
@@ -1032,12 +1033,6 @@ static int annotate_browser__run(struct annotate_browser *browser,
return key;
}
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt)
-{
- return symbol__tui_annotate(ms, evsel, hbt);
-}
-
int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
struct hist_browser_timer *hbt)
{
@@ -1046,11 +1041,12 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
SLang_init_tty(0, 0, 0);
SLtty_set_suspend_state(true);
- return map_symbol__tui_annotate(&he->ms, evsel, hbt);
+ return __hist_entry__tui_annotate(he, &he->ms, evsel, hbt);
}
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt)
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+ struct evsel *evsel,
+ struct hist_browser_timer *hbt)
{
struct symbol *sym = ms->sym;
struct annotation *notes = symbol__annotation(sym);
@@ -1064,6 +1060,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
.priv = ms,
.use_navkeypressed = true,
},
+ .he = he,
};
struct dso *dso;
int ret = -1, err;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d26b925e3d7f46af..55455c49faf01891 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2484,8 +2484,8 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
else
evsel = hists_to_evsel(browser->hists);
- err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
he = hist_browser__selected_entry(browser);
+ err = __hist_entry__tui_annotate(he, &act->ms, evsel, browser->hbt);
/*
* offer option to annotate the other branch source or target
* (if they exists) when returning from annotate
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 8b5131d257b01e3e..0f640e4871744262 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -471,18 +471,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
-#ifdef HAVE_SLANG_SUPPORT
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt);
-#else
-static inline int symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
- struct evsel *evsel __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused)
-{
- return 0;
-}
-#endif
-
void annotation_options__init(void);
void annotation_options__exit(void);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c64254088fc77246..11ae738772ca4f61 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -712,8 +712,9 @@ struct block_hist {
#include "../ui/keysyms.h"
void attr_to_script(char *buf, struct perf_event_attr *attr);
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt);
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+ struct evsel *evsel,
+ struct hist_browser_timer *hbt);
int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
struct hist_browser_timer *hbt);
@@ -741,9 +742,10 @@ int evlist__tui_browse_hists(struct evlist *evlist __maybe_unused,
{
return 0;
}
-static inline int map_symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
- struct evsel *evsel __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused)
+static inline int __hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
+ struct map_symbol *ms __maybe_unused,
+ struct evsel *evsel __maybe_unused,
+ struct hist_browser_timer *hbt __maybe_unused)
{
return 0;
}
--
2.50.0On Tue, Jul 15, 2025 at 10:00:47PM -0700, Namhyung Kim wrote:
> There are three different but similar functions for annotation on TUI.
Why the two initial __? Normally this is when its about work on some
data structure but the pointer to it isn't passed, but in this case the
first arg is a 'struct hist_entry *', so calling it
hist_entry__tui_annotate() would be right?
Well, there is already a hist_entry__tui_annotate(), that does some
term setup before calling this "new" __hist_entry__tui_annotate().
Looks confusing tho :-\
- Arnaldo
> Rename it to __hist_entry__tui_annotate() and make sure it passes 'he'.
> It's not used for now but it'll be needed for later use.
>
> Also remove map_symbol__tui_annotate() which was a simple wrapper.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 17 +++++++----------
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/util/annotate.h | 12 ------------
> tools/perf/util/hist.h | 12 +++++++-----
> 4 files changed, 15 insertions(+), 28 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 183902dac042ecb0..28ef146f29e8e742 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -27,6 +27,7 @@ struct annotate_browser {
> struct rb_node *curr_hot;
> struct annotation_line *selection;
> struct arch *arch;
> + struct hist_entry *he;
> bool searching_backwards;
> char search_bf[128];
> };
> @@ -557,7 +558,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
> target_ms.map = ms->map;
> target_ms.sym = dl->ops.target.sym;
> annotation__unlock(notes);
> - symbol__tui_annotate(&target_ms, evsel, hbt);
> + __hist_entry__tui_annotate(browser->he, &target_ms, evsel, hbt);
> sym_title(ms->sym, ms->map, title, sizeof(title), annotate_opts.percent_type);
> ui_browser__show_title(&browser->b, title);
> return true;
> @@ -1032,12 +1033,6 @@ static int annotate_browser__run(struct annotate_browser *browser,
> return key;
> }
>
> -int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> - struct hist_browser_timer *hbt)
> -{
> - return symbol__tui_annotate(ms, evsel, hbt);
> -}
> -
> int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> struct hist_browser_timer *hbt)
> {
> @@ -1046,11 +1041,12 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> SLang_init_tty(0, 0, 0);
> SLtty_set_suspend_state(true);
>
> - return map_symbol__tui_annotate(&he->ms, evsel, hbt);
> + return __hist_entry__tui_annotate(he, &he->ms, evsel, hbt);
> }
>
> -int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> - struct hist_browser_timer *hbt)
> +int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> + struct evsel *evsel,
> + struct hist_browser_timer *hbt)
> {
> struct symbol *sym = ms->sym;
> struct annotation *notes = symbol__annotation(sym);
> @@ -1064,6 +1060,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> .priv = ms,
> .use_navkeypressed = true,
> },
> + .he = he,
> };
> struct dso *dso;
> int ret = -1, err;
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index d26b925e3d7f46af..55455c49faf01891 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2484,8 +2484,8 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
> else
> evsel = hists_to_evsel(browser->hists);
>
> - err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
> he = hist_browser__selected_entry(browser);
> + err = __hist_entry__tui_annotate(he, &act->ms, evsel, browser->hbt);
> /*
> * offer option to annotate the other branch source or target
> * (if they exists) when returning from annotate
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 8b5131d257b01e3e..0f640e4871744262 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -471,18 +471,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
> int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
> int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
>
> -#ifdef HAVE_SLANG_SUPPORT
> -int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> - struct hist_browser_timer *hbt);
> -#else
> -static inline int symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
> - struct evsel *evsel __maybe_unused,
> - struct hist_browser_timer *hbt __maybe_unused)
> -{
> - return 0;
> -}
> -#endif
> -
> void annotation_options__init(void);
> void annotation_options__exit(void);
>
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index c64254088fc77246..11ae738772ca4f61 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -712,8 +712,9 @@ struct block_hist {
> #include "../ui/keysyms.h"
> void attr_to_script(char *buf, struct perf_event_attr *attr);
>
> -int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> - struct hist_browser_timer *hbt);
> +int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> + struct evsel *evsel,
> + struct hist_browser_timer *hbt);
>
> int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> struct hist_browser_timer *hbt);
> @@ -741,9 +742,10 @@ int evlist__tui_browse_hists(struct evlist *evlist __maybe_unused,
> {
> return 0;
> }
> -static inline int map_symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
> - struct evsel *evsel __maybe_unused,
> - struct hist_browser_timer *hbt __maybe_unused)
> +static inline int __hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
> + struct map_symbol *ms __maybe_unused,
> + struct evsel *evsel __maybe_unused,
> + struct hist_browser_timer *hbt __maybe_unused)
> {
> return 0;
> }
> --
> 2.50.0
Hi, welcome back!
On Wed, Jul 16, 2025 at 05:16:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Jul 15, 2025 at 10:00:47PM -0700, Namhyung Kim wrote:
> > There are three different but similar functions for annotation on TUI.
>
> Why the two initial __? Normally this is when its about work on some
> data structure but the pointer to it isn't passed, but in this case the
> first arg is a 'struct hist_entry *', so calling it
> hist_entry__tui_annotate() would be right?
>
> Well, there is already a hist_entry__tui_annotate(), that does some
> term setup before calling this "new" __hist_entry__tui_annotate().
>
> Looks confusing tho :-\
I thought it's a pattern to have a wrapper to deal with some boilerplate
code like this or locking, and have an internal function (with the "__"
prefix) to do the real work so that it can be called in a different
condition.
But I can rename if you don't feel comfortable with this. Any
suggestion?
Thanks,
Namhyung
>
> - Arnaldo
>
> > Rename it to __hist_entry__tui_annotate() and make sure it passes 'he'.
> > It's not used for now but it'll be needed for later use.
> >
> > Also remove map_symbol__tui_annotate() which was a simple wrapper.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/ui/browsers/annotate.c | 17 +++++++----------
> > tools/perf/ui/browsers/hists.c | 2 +-
> > tools/perf/util/annotate.h | 12 ------------
> > tools/perf/util/hist.h | 12 +++++++-----
> > 4 files changed, 15 insertions(+), 28 deletions(-)
> >
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 183902dac042ecb0..28ef146f29e8e742 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -27,6 +27,7 @@ struct annotate_browser {
> > struct rb_node *curr_hot;
> > struct annotation_line *selection;
> > struct arch *arch;
> > + struct hist_entry *he;
> > bool searching_backwards;
> > char search_bf[128];
> > };
> > @@ -557,7 +558,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
> > target_ms.map = ms->map;
> > target_ms.sym = dl->ops.target.sym;
> > annotation__unlock(notes);
> > - symbol__tui_annotate(&target_ms, evsel, hbt);
> > + __hist_entry__tui_annotate(browser->he, &target_ms, evsel, hbt);
> > sym_title(ms->sym, ms->map, title, sizeof(title), annotate_opts.percent_type);
> > ui_browser__show_title(&browser->b, title);
> > return true;
> > @@ -1032,12 +1033,6 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > return key;
> > }
> >
> > -int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > - struct hist_browser_timer *hbt)
> > -{
> > - return symbol__tui_annotate(ms, evsel, hbt);
> > -}
> > -
> > int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> > struct hist_browser_timer *hbt)
> > {
> > @@ -1046,11 +1041,12 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> > SLang_init_tty(0, 0, 0);
> > SLtty_set_suspend_state(true);
> >
> > - return map_symbol__tui_annotate(&he->ms, evsel, hbt);
> > + return __hist_entry__tui_annotate(he, &he->ms, evsel, hbt);
> > }
> >
> > -int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > - struct hist_browser_timer *hbt)
> > +int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > + struct evsel *evsel,
> > + struct hist_browser_timer *hbt)
> > {
> > struct symbol *sym = ms->sym;
> > struct annotation *notes = symbol__annotation(sym);
> > @@ -1064,6 +1060,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > .priv = ms,
> > .use_navkeypressed = true,
> > },
> > + .he = he,
> > };
> > struct dso *dso;
> > int ret = -1, err;
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index d26b925e3d7f46af..55455c49faf01891 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -2484,8 +2484,8 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
> > else
> > evsel = hists_to_evsel(browser->hists);
> >
> > - err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
> > he = hist_browser__selected_entry(browser);
> > + err = __hist_entry__tui_annotate(he, &act->ms, evsel, browser->hbt);
> > /*
> > * offer option to annotate the other branch source or target
> > * (if they exists) when returning from annotate
> > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> > index 8b5131d257b01e3e..0f640e4871744262 100644
> > --- a/tools/perf/util/annotate.h
> > +++ b/tools/perf/util/annotate.h
> > @@ -471,18 +471,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
> > int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
> > int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
> >
> > -#ifdef HAVE_SLANG_SUPPORT
> > -int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > - struct hist_browser_timer *hbt);
> > -#else
> > -static inline int symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
> > - struct evsel *evsel __maybe_unused,
> > - struct hist_browser_timer *hbt __maybe_unused)
> > -{
> > - return 0;
> > -}
> > -#endif
> > -
> > void annotation_options__init(void);
> > void annotation_options__exit(void);
> >
> > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> > index c64254088fc77246..11ae738772ca4f61 100644
> > --- a/tools/perf/util/hist.h
> > +++ b/tools/perf/util/hist.h
> > @@ -712,8 +712,9 @@ struct block_hist {
> > #include "../ui/keysyms.h"
> > void attr_to_script(char *buf, struct perf_event_attr *attr);
> >
> > -int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > - struct hist_browser_timer *hbt);
> > +int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > + struct evsel *evsel,
> > + struct hist_browser_timer *hbt);
> >
> > int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
> > struct hist_browser_timer *hbt);
> > @@ -741,9 +742,10 @@ int evlist__tui_browse_hists(struct evlist *evlist __maybe_unused,
> > {
> > return 0;
> > }
> > -static inline int map_symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
> > - struct evsel *evsel __maybe_unused,
> > - struct hist_browser_timer *hbt __maybe_unused)
> > +static inline int __hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
> > + struct map_symbol *ms __maybe_unused,
> > + struct evsel *evsel __maybe_unused,
> > + struct hist_browser_timer *hbt __maybe_unused)
> > {
> > return 0;
> > }
> > --
> > 2.50.0
© 2016 - 2025 Red Hat, Inc.