arch_topology: Build cacheinfo from primary CPU

Hello Pierre Gondois,

The patch 5944ce092b97: "arch_topology: Build cacheinfo from primary CPU" from Jan 4, 2023, leads to the following Smatch static checker warning:

drivers/base/cacheinfo.c:440 fetch_cache_info() error: uninitialized symbol 'levels'.
drivers/base/cacheinfo.c:447 fetch_cache_info() error: uninitialized symbol 'split_levels'.

drivers/base/cacheinfo.c

     424 int fetch_cache_info(unsigned int cpu)
     425 {
     426         struct cpu_cacheinfo *this_cpu_ci;
     427         unsigned int levels, split_levels;
     428         int ret;
     429
     430         if (acpi_disabled) {
     431                 ret = init_of_cache_level(cpu);
     432                 if (ret < 0)
     433                         return ret;
     434         } else {
     435                 ret = acpi_get_cache_info(cpu, &levels, &split_levels);
     436                 if (ret < 0)
     437                         return ret;

Apparently, I must have CONFIG_ACPI_PPTT disabled.

     438
     439                 this_cpu_ci = get_cpu_cacheinfo(cpu);
--> 440                 this_cpu_ci->num_levels = levels;
                                                   ^^^^^^

Unititialized.

     441                 /*
     442                  * This assumes that:
     443                  * - there cannot be any split caches (data/instruction)
     444                  *   above a unified cache
     445                  * - data/instruction caches come by pair
     446                  */
     447                 this_cpu_ci->num_leaves = levels + split_levels;
     448         }
     449         if (!cache_leaves(cpu))
     450                 return -ENOENT;
     451
     452         return allocate_cache_info(cpu);
     453 }

regards,
dan carpenter

Hello Dan,
The 'levels' and 'split_levels' variables are initialized through their addresses when necessary, so I believe the warning can be ignored.

If you still want to have the variables initialized, please let me know and I will send a patch,

Quoted 3 lines · Pierre Gondois

Hello Dan,
The 'levels' and 'split_levels' variables are initialized through their addresses when necessary, so I believe the warning can be ignored.

I don't understand what "initialized through their addresses when necessary" means. If you have CONFIG_ACPI_PPTT turned off as I do then acpi_get_cache_info() is a no-op so it's not initializing anything.

Quoted 4 lines

If you still want to have the variables initialized, please let me know and I will send a patch,

I feel like I have this kind of discussion a lot. I don't know why people don't want to initialize their variables to zero.

1) It doesn't affect runtime on modern distros because they use the CONFIG_ option to zero stack variables.

2) If they don't then syzbot will detect it at runtime. It will probably take a year or two. That will set off a bunch of emails. Meanwhile you will have joined an exciting new start up and won't be around. No one else will remember how this code works.

regards,
dan carpenter