linux-kernel - Re: [PATCH v2 2/2] hwmon: iio: Add labels from IIO channels

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]

Message-ID: <11e054b6-a116-417b-9112-f32a8bc6b121@roeck-us.net>
Date: Mon, 24 Jun 2024 13:05:35 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Sean Anderson <sean.anderson@...ux.dev>,
 Jonathan Cameron <jic23@...nel.org>, Jean Delvare <jdelvare@...e.com>,
 linux-iio@...r.kernel.org, linux-hwmon@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Lars-Peter Clausen <lars@...afoo.de>
Subject: Re: [PATCH v2 2/2] hwmon: iio: Add labels from IIO channels

On 6/24/24 12:34, Sean Anderson wrote:
> On 6/24/24 14:47, Guenter Roeck wrote:
>> On 6/24/24 10:46, Sean Anderson wrote:
>>> Add labels from IIO channels to our channels. This allows userspace to
>>> display more meaningful names instead of "in0" or "temp5".
>>>
>>> Although lm-sensors gracefully handles errors when reading channel
>>> labels, the ABI says the label attribute
>>>
>>>> Should only be created if the driver has hints about what this voltage
>>>> channel is being used for, and user-space doesn't.
>>>
>>> Therefore, we test to see if the channel has a label before
>>> creating the attribute.
>>>
>>
>> FWIW, complaining about an ABI really does not belong into a commit
>> message. Maybe you and lm-sensors don't care about error returns when
>> reading a label, but there are other userspace applications which may
>> expect drivers to follow the ABI. Last time I checked, the basic rule
>> was still "Don't break userspace", and that doesn't mean "it's ok to
>> violate / break an ABI as long as no one notices".
> 
> This isn't complaining about the ABI, just documenting the reason it was
> done this way...
> 

That a patch is implemented to follow its ABI is not worth mentioning
in the commit message. You _do_ mention it, and added "Although lm-sensors
gracefully ... ". So, from my perspective it is complaining about the ABI,
unless you think that pretty much all patches should include "this is done
to comply with the ABI, even though <some userspace application> is fine
with violating it".

Never mind though, I gave it my Acked-by:, and consider the issue closed.

Guenter