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.