Re: [PATCH v5 11/23] perf evlist: Change env variable to session

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Jun 27, 2025 at 09:50:05PM -0700, Ian Rogers wrote:
> The session holds a perf_env pointer env. In UI code container_of is
> used to turn the env to a session, but this assumes the session
> header's env is in use. Rather than a dubious container_of, hold the
> session in the evlist and derive the env from the session with
> evsel__env, perf_session__env, etc.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-report.c      |  6 +++++-
>  tools/perf/builtin-script.c      |  2 +-
>  tools/perf/builtin-top.c         |  2 +-
>  tools/perf/tests/topology.c      |  1 +
>  tools/perf/ui/browser.h          |  4 ++--
>  tools/perf/ui/browsers/header.c  |  4 +---
>  tools/perf/ui/browsers/hists.c   |  2 +-
>  tools/perf/util/amd-sample-raw.c |  2 +-
>  tools/perf/util/arm-spe.c        |  2 +-
>  tools/perf/util/evlist.h         |  2 +-
>  tools/perf/util/evsel.c          | 12 +++++++++---
>  tools/perf/util/evsel.h          |  1 +
>  tools/perf/util/header.c         |  2 +-
>  tools/perf/util/s390-cpumsf.c    |  2 +-
>  tools/perf/util/sample-raw.c     |  7 ++++---
>  tools/perf/util/sample-raw.h     |  2 +-
>  tools/perf/util/session.c        |  4 +++-
>  17 files changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 704576e46e4b..ada8e0166c78 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1274,6 +1274,8 @@ static int process_attr(const struct perf_tool *tool __maybe_unused,
>  			union perf_event *event,
>  			struct evlist **pevlist)
>  {
> +	struct perf_session *session;
> +	struct perf_env *env;
>  	u64 sample_type;
>  	int err;
>  
> @@ -1286,7 +1288,9 @@ static int process_attr(const struct perf_tool *tool __maybe_unused,
>  	 * on events sample_type.
>  	 */
>  	sample_type = evlist__combined_sample_type(*pevlist);
> -	callchain_param_setup(sample_type, perf_env__arch((*pevlist)->env));
> +	session = (*pevlist)->session;
> +	env = perf_session__env(session);
> +	callchain_param_setup(sample_type, perf_env__arch(env));
>  	return 0;
>  }
>  
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 8a452353c867..2c25eda4be26 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2535,7 +2535,7 @@ static int process_attr(const struct perf_tool *tool, union perf_event *event,
>  	 * on events sample_type.
>  	 */
>  	sample_type = evlist__combined_sample_type(evlist);
> -	callchain_param_setup(sample_type, perf_env__arch((*pevlist)->env));
> +	callchain_param_setup(sample_type, perf_env__arch(perf_session__env(scr->session)));
>  
>  	/* Enable fields for callchain entries */
>  	if (symbol_conf.use_callchain &&
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 84b223a94dcf..72f9be5a3b30 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1654,7 +1654,6 @@ int cmd_top(int argc, const char **argv)
>  			"Couldn't read the cpuid for this machine: %s\n",
>  			str_error_r(errno, errbuf, sizeof(errbuf)));
>  	}
> -	top.evlist->env = &perf_env;
>  
>  	argc = parse_options(argc, argv, options, top_usage, 0);
>  	if (argc)
> @@ -1822,6 +1821,7 @@ int cmd_top(int argc, const char **argv)
>  		perf_top__update_print_entries(&top);
>  		signal(SIGWINCH, winch_sig);
>  	}
> +	top.session->env = &perf_env;

Looks like it's accessing top.session before it's created?

Thanks,
Namhyung

>  
>  	top.session = perf_session__new(NULL, NULL);
>  	if (IS_ERR(top.session)) {