Re: [PATCH] tools/lib/perf: Fix -Werror=alloc-size-larger-than in cpumap.c
From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2025-05-13 21:13:21
Also in:
linux-perf-users
On Fri, May 02, 2025 at 01:14:32PM +0530, Mukesh Kumar Chaurasiya wrote:
On Fri, Apr 25, 2025 at 02:46:43PM -0300, Arnaldo Carvalho de Melo wrote:quoted
Maybe that max() call in perf_cpu_map__intersect() somehow makes the compiler happy.
quoted
And in perf_cpu_map__alloc() all calls seems to validate it.
quoted
Like:
quoted
+++ b/tools/lib/perf/cpumap.c@@ -411,7 +411,7 @@ int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other) } tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other); - tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); + tmp_cpus = calloc(tmp_len, sizeof(struct perf_cpu)); if (!tmp_cpus) return -ENOMEM;
quoted
⬢ [acme@toolbx perf-tools-next]$
quoted
And better, do the max size that the compiler is trying to help us catch?
Isn't it better to use perf_cpu_map__nr. That should fix this problem.
Maybe, have you tried it?
One question I have, in perf_cpu_map__nr, the function is returning 1 in case *cpus is NULL. Is it ok to do that? wouldn't it cause problems?
Indeed this better be documented, as by just looking at:
int perf_cpu_map__nr(const struct perf_cpu_map *cpus)
{
return cpus ? __perf_cpu_map__nr(cpus) : 1;
}
It really doesn't make much sense to say that a NULL map has one entry.
But the next functions are:
bool perf_cpu_map__has_any_cpu_or_is_empty(const struct perf_cpu_map *map)
{
return map ? __perf_cpu_map__cpu(map, 0).cpu == -1 : true;
}
bool perf_cpu_map__is_any_cpu_or_is_empty(const struct perf_cpu_map *map)
{
if (!map)
return true;
return __perf_cpu_map__nr(map) == 1 && __perf_cpu_map__cpu(map, 0).cpu == -1;
}
bool perf_cpu_map__is_empty(const struct perf_cpu_map *map)
{
return map == NULL;
}
So it seems that a NULL cpu map means "any/all CPU) and a map with just
one entry would have as its content "-1" that would mean "any/all CPU".
Ian did work on trying to simplify/clarify this, so maybe he can chime
in :-)
- Arnaldo