Re: [PATCH v1 13/13] perf python: Fix MetricGroup return type in perf.pyi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but local references to the Python objects 'key' and 'value' in `pyrf__metrics_cb` are leaked on the success path.
--

commit 0b60b233b211850d4730f46ab4faf2da85366d5f
Author: Ian Rogers <irogers@google.com>

perf python: Fix MetricGroup return type in perf.pyi

The metrics() function can return a dictionary where the value is either
a string or a list of strings, so the type signature in the stub file
should be Union[str, List[str]].

> diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
> index f266c9baffd01..58b0b3ed819d8 100644
> --- a/tools/perf/python/perf.pyi
> +++ b/tools/perf/python/perf.pyi
[ ... ]
> @@ -12,7 +12,7 @@ def config_get(name: str) -> Optional[str]:
>      """
>      ...
>  
> -def metrics() -> List[Dict[str, str]]:
> +def metrics() -> List[Dict[str, Union[str, List[str]]]]:

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the return
type update for metrics(), I noticed a potential resource leak in its C
implementation.

Looking at tools/perf/util/python.c:pyrf__metrics_cb():

	if (!value || PyDict_SetItem(dict, key, value) != 0) {
		Py_XDECREF(key);
		Py_XDECREF(value);
		Py_XDECREF(dict);
		return -ENOMEM;
	}

	if (!add_to_dict(dict, "MetricName", pm->metric_name) ||

Does this leak the local references to the Python objects 'key' and 'value'
on the success path?

Since PyDict_SetItem() does not steal references, it appears that 'key' and
'value' might need a Py_DECREF() after a successful insertion. Without it,
a script repeatedly calling perf.metrics() could continually leak memory.

>      """Get a list of available metrics.
>  
>      Returns:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623053515.2677888-1-irogers@google.com?part=13