Re: potential return of uninitialized variable ret in function skx_upi_topology_cb

[Alexander Antonov <alexander.antonov@linux.intel.com>]

Hi Colin,
Thank you so much for reporting the issue. Seems fix wasn't picked up 
before, I will resend it.

Thank you


On 7/27/2023 3:55 PM, Colin King (gmail) wrote:
> Hi,
>
> Static analysis with cppcheck has detected a potential return of an 
> uninitialized variable in function skx_upi_topology_cb. The issue was 
> introduced with commit:
>
> commit 4cfce57fa42d277497cd2c425021312eae2f223c
> Author: Alexander Antonov <alexander.antonov@linux.intel.com>
> Date:   Thu Nov 17 12:28:28 2022 +0000
>
>     perf/x86/intel/uncore: Enable UPI topology discovery for Skylake 
> Server
>
> static int skx_upi_topology_cb(struct intel_uncore_type *type, int 
> segment,
>                                 int die, u64 cpu_bus_msr)
> {
>         int idx, ret;
>
> ^^ ret is not initialized
>
>         struct intel_uncore_topology *upi;
>         unsigned int devfn;
>         struct pci_dev *dev = NULL;
>         u8 bus = cpu_bus_msr >> (3 * BUS_NUM_STRIDE);
>
>         for (idx = 0; idx < type->num_boxes; idx++) {
>                 upi = &type->topology[die][idx];
>                 devfn = PCI_DEVFN(SKX_UPI_REGS_ADDR_DEVICE_LINK0 + idx,
>                                   SKX_UPI_REGS_ADDR_FUNCTION);
>                 dev = pci_get_domain_bus_and_slot(segment, bus, devfn);
>
> ^^ dev may be null, so ret is never assigned
>
>                 if (dev) {
>                         ret = upi_fill_topology(dev, upi, idx);
>                         if (ret)
>                                 break;
>                 }
>         }
>
>         pci_dev_put(dev);
>         return ret;
> }
>
> I suspect this probably is very unlikely, but it would be useful to 
> have ret initialized to some value to avoid garbage being returned. 
> Not sure what the best value is to set as as default in this corner case.
>
> Colin