Re: [PATCH] perf cpumap: Make counter as unsigned ints

[Khem Raj <raj.khem@gmail.com>]

On Tue, Jan 24, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Jan 23, 2023 at 1:13 PM Khem Raj <raj.khem@gmail.com> wrote:
> >
> > These are loop counters which is inherently unsigned. Therefore make
> > them unsigned. Moreover it also fixes alloc-size-larger-than
> > error with gcc-13, where malloc can be called with (-1) due to tmp_len
> > being an int type.
> >
> > Fixes
> > | cpumap.c:366:20: error: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
> > |   366 |         tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
> > |       |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/lib/perf/cpumap.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
> > index 6cd0be7c1bb4..d960880dd903 100644
> > --- a/tools/lib/perf/cpumap.c
> > +++ b/tools/lib/perf/cpumap.c
> > @@ -351,8 +351,8 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> >                                          struct perf_cpu_map *other)
> >  {
> >         struct perf_cpu *tmp_cpus;
> > -       int tmp_len;
> > -       int i, j, k;
> > +       unsigned int tmp_len;
> > +       unsigned int i, j, k;
> >         struct perf_cpu_map *merged;
> >
> >         if (perf_cpu_map__is_subset(orig, other))
> > @@ -369,7 +369,7 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> >
> >         /* Standard merge algorithm from wikipedia */
> >         i = j = k = 0;
> > -       while (i < orig->nr && j < other->nr) {
> > +       while (i < (unsigned int)orig->nr && j < (unsigned int)other->nr) {
>
> Rather than cast 'nr' why not add the unsigned to the declaration?
>

if type of nr is changed from int to unsigned int, this will ensue a
lot of changes including function signature changes, I am not
sure if that is the best approach.

> Thanks,
> Ian
>
> >                 if (orig->map[i].cpu <= other->map[j].cpu) {
> >                         if (orig->map[i].cpu == other->map[j].cpu)
> >                                 j++;
> > @@ -378,10 +378,10 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> >                         tmp_cpus[k++] = other->map[j++];
> >         }
> >
> > -       while (i < orig->nr)
> > +       while (i < (unsigned int)orig->nr)
> >                 tmp_cpus[k++] = orig->map[i++];
> >
> > -       while (j < other->nr)
> > +       while (j < (unsigned int)other->nr)
> >                 tmp_cpus[k++] = other->map[j++];
> >         assert(k <= tmp_len);
> >
> > --
> > 2.39.1
> >