All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ian Rogers" <irogers@google.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 01/10] perf jevents: Store raw bytes in BigCString instead of C-escaped strings
Date: Fri, 13 Nov 2026 21:21:54 +0000	[thread overview]
Message-ID: <sashiko-outbox-213712@kernel.org> (raw)
In-Reply-To: <20261113210822.3112700-2-irogers@google.com>

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf jevents: incompatible pmu_name redefinition breaks mypy
  build
--

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

perf jevents: Store raw bytes in BigCString instead of C-escaped strings

This commit changes BigCString and _metricgroups to store raw NUL-delimited
UTF-8 bytes instead of C-escaped strings, escaping them into C string
literals only when emitting C source.

> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 080e04b51b8c5..0721c8b2551c3 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py

[ ... ]

> @@ -556,11 +550,12 @@ def print_pending_events() -> None:
>  static const struct pmu_table_entry {_pending_events_tblname}[] = {{
>  """)
>    for (pmu, tbl_pmu) in sorted(pmus):
> -    pmu_name = f"{pmu}\\000"
> +    pmu_name = (pmu + '\0').encode('utf-8')
> +    pmu_comment = bytes_to_c_literal(pmu_name).replace('*/', r'\*\/')
>      _args.output_file.write(f"""\t{{
>  \t\t.entries = {_pending_events_tblname}_{tbl_pmu},
>  \t\t.num_entries = ARRAY_SIZE({_pending_events_tblname}_{tbl_pmu}),
> -\t\t.pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_name} */ }},
> +\t\t.pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_comment} */ }},
>  \t}},
>  """)
>    _args.output_file.write('};\n\n')
> @@ -612,11 +607,12 @@ def print_pending_metrics() -> None:
>  static const struct pmu_table_entry {_pending_metrics_tblname}[] = {{
>  """)
>    for (pmu, tbl_pmu) in sorted(pmus):
> -    pmu_name = f"{pmu}\\000"
> +    pmu_name = (pmu + '\0').encode('utf-8')
> +    pmu_comment = bytes_to_c_literal(pmu_name).replace('*/', r'\*\/')
>      _args.output_file.write(f"""\t{{
>  \t\t.entries = {_pending_metrics_tblname}_{tbl_pmu},
>  \t\t.num_entries = ARRAY_SIZE({_pending_metrics_tblname}_{tbl_pmu}),
> -\t\t.pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_name} */ }},
> +\t\t.pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_comment} */ }},
>  \t}},
>  """)
>    _args.output_file.write('};\n\n')

[Severity: Medium]
Does reassigning bytes to pmu_name in print_pending_events() and
print_pending_metrics() break the default mypy build target?

Earlier in both functions, pmu_name is assigned a str value:

tools/perf/pmu-events/jevents.py:print_pending_events() {
    ...
      pmu_name = event.pmu.replace(',', '_')
    ...
}

Rebinding pmu_name to bytes and passing it to bytes_to_c_literal() and
_bcs.offsets[] causes mypy to fail with incompatible assignment, argument
type, and index type errors, breaking git bisect until the later commit
"perf jevents: Store PMU names as string literals in struct
pmu_table_entry" removes these assignments.  Using a separate variable name
such as pmu_name_bytes (like mgroup_bytes in preprocess_one_file()) avoids
the type conflict.

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

  reply	other threads:[~2026-11-13 21:21 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 21:08 [PATCH 00/10] perf jevents: Block-indexed string compression and default JEVENTS_ARCH=all Ian Rogers
2026-11-13 21:08 ` [PATCH 01/10] perf jevents: Store raw bytes in BigCString instead of C-escaped strings Ian Rogers
2026-11-13 21:21   ` sashiko-bot [this message]
2026-11-13 21:08 ` [PATCH 02/10] perf jevents: Sort BigCString entries alphabetically Ian Rogers
2026-11-13 21:12   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 03/10] perf jevents: Store PMU names as string literals in struct pmu_table_entry Ian Rogers
2026-11-13 21:13   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 04/10] perf jevents: Encapsulate big_c_string lookups behind pmu_events__get_string() Ian Rogers
2026-11-13 21:12   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 05/10] perf jevents: Add block-indexed string packing and --compress=zlib Ian Rogers
2026-11-13 21:16   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 06/10] perf jevents: Add --compress=lzma block compression backend Ian Rogers
2026-11-13 21:13   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 07/10] perf jevents: Add --compress=zstd " Ian Rogers
2026-11-13 21:13   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 08/10] perf build: Enable jevents string compression when available Ian Rogers
2026-11-13 21:17   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 09/10] perf tests pmu-events: Test metricgroup descriptions and concurrent lookups Ian Rogers
2026-11-13 21:20   ` sashiko-bot
2026-11-13 21:08 ` [PATCH 10/10] perf build: Default JEVENTS_ARCH to all Ian Rogers
2026-11-13 21:21   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-213712@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=irogers@google.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.