Re: [PATCH 11/11] libperf: Document code simplification case for widening struct perf_cpu

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Mon, Jun 08, 2026 at 03:04:40PM -0700, Ian Rogers wrote:
> On Mon, Jun 8, 2026 at 1:50 PM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] The documentation added to `tools/lib/perf/TODO` incorrectly states that narrowing values > 32767 to `int16_t` wraps them to 'small positive numbers'. In two's complement arithmetic, values from 32768 to 65535 wrap to negative numbers (-32768 to -1) when truncated to a signed 16-bit integer.
> > --
> >
> > commit e8f5b26d672396c3173a77e6aaeee613f6d309bd
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > libperf: Document code simplification case for widening struct perf_cpu
> >
> > Add a bullet point to the libperf ABI TODO explaining the code
> > simplification benefit of widening struct perf_cpu.cpu from int16_t
> > to int.
> >
> > > diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
> > > index 486dd95dc5720..1a3644aa1f38d 100644
> > > --- a/tools/lib/perf/TODO
> > > +++ b/tools/lib/perf/TODO
> > > @@ -11,6 +11,13 @@ together.
> > >       (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
> > >       growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
> > >       safety net.
> > > +   - Code simplification: the int16_t forces defensive truncation
> 
> I think Sashiko just wants this to be uint16_t :-)

That would buy us some more decades, maybe. :-)

- Arnaldo
 
> Thanks,
> Ian
> 
> > > +     checks at every boundary where a wider CPU index (int from
> > > +     sample->cpu, al->cpu, etc.) is narrowed into struct perf_cpu.
> > > +     Without these checks, values > 32767 silently wrap to small
> > > +     positive numbers, bypassing bounds validation.  Widening to int
> >        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > [Severity: Low]
> > This isn't a bug, but is the arithmetic description accurate here?
> >
> > In two's complement arithmetic, truncating values between 32768 and 65535
> > into a signed 16-bit integer (int16_t) wraps them to negative numbers
> > (-32768 to -1), rather than small positive numbers.
> >
> > The overall conclusion remains correct since negative values trivially
> > bypass bounds checks, but the description of the wrapping behavior appears
> > mathematically inaccurate.
> >
> > > +     eliminates this entire class of silent truncation bugs and
> > > +     removes the need for the INT16_MAX clamp in set_max_cpu_num().
> > >     - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=11
> >