On 08/23/2018 05:24 AM, Michael Ellerman wrote:
> Hi Breno,
>
> Breno Leitao <lei...@debian.org> writes:
>> Function xive_native_get_ipi() might uses chip_id without it being
>> initialized. This gives the following error on 'smatch' tool:
>>
>> error: uninitialized symbol 'chip_id'
>
> Which is correct, it can be used uninitialised. I'm surprised GCC
> doesn't warn about it.
>
>> This patch simply sets chip_id initial value to 0.
>
> I'd prefer we fixed it differently, by explicitly initialising to zero
> at the appropriate place in the code.
>
>> diff --git a/arch/powerpc/sysdev/xive/native.c
>> b/arch/powerpc/sysdev/xive/native.c
>> index 311185b9960a..fc56673a3c0f 100644
>> --- a/arch/powerpc/sysdev/xive/native.c
>> +++ b/arch/powerpc/sysdev/xive/native.c
>> @@ -239,7 +239,7 @@ static bool xive_native_match(struct device_node *node)
>> static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>> {
>> struct device_node *np;
>> - unsigned int chip_id;
>> + unsigned int chip_id = 0;
>> s64 irq;
>>
>> /* Find the chip ID */
>
> The current code is:
>
> /* Find the chip ID */
> np = of_get_cpu_node(cpu, NULL);
> if (np) {
> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
> chip_id = 0;
> }
>
> Where if np is NULL then we don't initialise chip_id.
>
> Which could be:
>
> np = of_get_cpu_node(cpu, NULL);
> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
> chip_id = 0;
>
> Because of_property_read_u32() will just return an error if np is NULL.
>
> It's also missing an of_node_put() of np, you should do a separate patch
> to fix that. You can just do it unconditionally after the
> of_property_read_u32().
I think we can simply get rid of the OF code under xive_native_get_ipi()
and use xc->chip_id instead. It should be safe to use as xive_prepare_cpu()
should have initialized ->chip_id by the time xive_native_get_ipi() is
called.
Cheers,
C.
