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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Mar 2022 14:03:07 +0100
From:   Agathe Porte <agathe.porte@...ia.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>
Cc:     Jean Delvare <jdelvare@...e.com>, Rob Herring <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Adamski, Krzysztof (Nokia - PL/Wroclaw)" 
        <krzysztof.adamski@...ia.com>
Subject: Re: [PATCH v7 2/2] hwmon: Add driver for Texas Instruments TMP464 and
 TMP468

Hi Guenter,

Le 3/15/2022 à 2:22 AM, Guenter Roeck a écrit :
> 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.

Thanks for the explanation. I agree that there is no problem on this point.

> 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.

I have to disagree here. Because something has always (not) be done in 
the past should not be a reason to (not) do it in the future out of pure 
habit. I did not suggest to add the (void) casts in existing code: I 
agree it would be a burden with no real added value.

But making static analyzers happy seems justified *for new code*. It 
also makes *other developers* more confident, because with the cast we 
are sure that not checking the return value is very intentional.

Please enlighten me if there are any downsides that I did not think of 
and that would block this one-line change.

Best regards,

Agathe.