Re: [patch 2/2 v2] leds: netxbig: silence a static checker warning

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 04/10/2015 09:52 PM, Simon Guinot wrote:
> On Fri, Apr 10, 2015 at 06:50:34PM +0300, Dan Carpenter wrote:
>> I've looked at this some more.  Most of the places which call
>> of_property_read_u32_index() check the return code.  The ones that don't
>> mostly initialize their values going in.  The remainder introduce static
>> checker warnings like:
>>
>> 	drivers/clk/ti/divider.c:472 ti_clk_get_div_table()
>> 	error: potentially using uninitialized 'val'.
>>
>> These warnings cause me pain.  It calls of_get_property() earlier so
>> it won't return -EINVAL.  I don't know if it can return -ENODATA or
>> -EOVERFLOW?
>>
>> I guess not.
>
> I think it can't. Above, we are calling of_property_count_u32_elems() to
> count the number of u32 elements in the "timers" property. After we are
> ensuring that there is three u32 elements available per timer. That's
> why the return codes for the three of_property_read_u32_index() calls
> are not checked.

After looking at Documentation/devicetree/bindings/leds/leds-netxbig.txt
I noticed inconsistency: timers property is defined as required, but
the comment over the call to of_property_count_u32_elems says that it
is optional.

I think that DT documentation should be changed to make the property
optional. How do you think?

Besides, I am wondering if we shouldn't check if the values read are
sane? In such a case initializing delay_on and delay_off to 0 would be
useful. We could check if both delays don't equal 0, which could happen
if the of_property_read_u32_index returned negative value because of
providing values out of bounds or not numerical values.

-- 
Best Regards,
Jacek Anaszewski