Re: [PATCH v7 2/2] hwmon: Add driver for Texas Instruments TMP464 and TMP468

[Guenter Roeck <linux@roeck-us.net>]

Hi Agathe,

On 3/14/22 08:46, Agathe Porte wrote:
> Hi,
> 
> Le 2/22/2022 à 11:36 PM, Guenter Roeck a écrit :
>>   of_property_read_string(child,"label", &data->channel[channel].label);
> 
> Upon trying to merge v7 in our codebase, our static analyzer tool detected that the return code of this function was not checked.
> 
> As I guess putting a label is optional, maybe we should add a `(void)` on the same line just before the function call to clearly indicate that not checking the return value is intentional and that it is not a coding mistake?
> 
> EDIT: As I was reading the implementation of of_property_read_string [1], it will not touch the destination string in case of error. Which means that labels may sit uninitialized and contain garbage data?
> 

Thanks for the feedback.

If of_property_read_string() returns an error, it will not set the pointer
to &data->channel[channel].label, which by default is NULL because the
data structure was allocated with devm_kzalloc(). That means tmp464_is_visible()
will disable the label attribute. I don't see a problem with the current
code.

There are lots of examples in the kernel where the return value from
of_property_read_string() is silently ignored. Not a single one of
those uses a (void) typecast. I don't really want to start making
such changes just to make static analyzers happy.

Thanks,
Guenter