Re: [PATCH] perf stat: allocation check when calculating cache instance ID

[Yunseong Kim]

On 5/29/24 3:56 오전, Namhyung Kim wrote:
> Hello Yunseong,
> 
> On Mon, May 27, 2024 at 2:06 AM <yskelg@xxxxxxxxx> wrote:
>>
>> From: Yunseong Kim <yskelg@xxxxxxxxx>
>>
>> Adds an allocation check for cpu_map before perf_cpu_map__min() accessing
> 
> I think perf_cpu_map__min() works well with NULL.
> 
> Thanks,
> Namhyung

Thank you for your code review Namhyung.

I'll try to find a more useful improvement and patch it next time!

Warm Regards,
Yunseong Kim

>>
>> Signed-off-by: Yunseong Kim <yskelg@xxxxxxxxx>
>> ---
>>  tools/perf/builtin-stat.c | 24 +++++++++++++-----------
>>  1 file changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 35f79b48e8dc..1f238824abb2 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1291,20 +1291,22 @@ static struct option stat_options[] = {
>>   */
>>  static int cpu__get_cache_id_from_map(struct perf_cpu cpu, char *map)
>>  {
>> -       int id;
>> +       int id = cpu.cpu;
>>         struct perf_cpu_map *cpu_map = perf_cpu_map__new(map);
>>
>> -       /*
>> -        * If the map contains no CPU, consider the current CPU to
>> -        * be the first online CPU in the cache domain else use the
>> -        * first online CPU of the cache domain as the ID.
>> -        */
>> -       id = perf_cpu_map__min(cpu_map).cpu;
>> -       if (id == -1)
>> -               id = cpu.cpu;
>> +       if (cpu_map) {
>> +               /*
>> +                * If the map contains no CPU, consider the current CPU to
>> +                * be the first online CPU in the cache domain else use the
>> +                * first online CPU of the cache domain as the ID.
>> +                */
>> +               id = perf_cpu_map__min(cpu_map).cpu;
>> +               if (id == -1)
>> +                       id = cpu.cpu;
>>
>> -       /* Free the perf_cpu_map used to find the cache ID */
>> -       perf_cpu_map__put(cpu_map);
>> +               /* Free the perf_cpu_map used to find the cache ID */
>> +               perf_cpu_map__put(cpu_map);
>> +       }
>>
>>         return id;
>>  }
>> --
>> 2.34.1
>>