Re: [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unconditional access of `j.metric_expr.ToPerfJson()` can raise `AttributeError: 'NoneType' object has no attribute 'ToPerfJson'` in `jevents.py`, causing a build failure.
--

--- Patch [1]: [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order ---
commit ef069542148ff8d19abeda05cc13982058eb11d7
Author: Ian Rogers <irogers@google.com>

perf jevents: Add more components to the metric sorting order

This commit modifies the sorting order for metrics in the jevents.py script
by including expressions and descriptions. The goal is to prevent
non-deterministic builds that were occurring when metrics were sorted only
by name.

Closes: https://lore.kernel.org/linux-perf-users/20260706175624.692736-1-nazar.kazakov@codethink.co.uk/

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 376dc2d241621..3c6cfeefbd5dc 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -570,13 +570,14 @@ static const struct pmu_table_entry {_pending_events_tblname}[] = {{
>  def print_pending_metrics() -> None:
>    """Optionally close metrics table."""
>  
> -  def metric_cmp_key(j: JsonEvent) -> Tuple[bool, str, str]:
> +  def metric_cmp_key(j: JsonEvent) -> Tuple[str, str, str, str]:
>      def fix_none(s: Optional[str]) -> str:
>        if s is None:
>          return ''
>        return s
>  
> -    return (j.desc is not None, fix_none(j.pmu), fix_none(j.metric_name))
> +    return (fix_none(j.pmu), fix_none(j.metric_name), j.metric_expr.ToPerfJson(),
> +            fix_none(j.desc))

[Severity: Low]
Will this crash with an AttributeError if an event lacks a metric expression?

If a parsed JSON event specifies a MetricName but omits MetricExpr,
j.metric_expr will be None. Calling j.metric_expr.ToPerfJson() in this case
would cause the build script to fail.

Would it be safer to check if j.metric_expr is None before calling
ToPerfJson()?

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