linux-kernel - Re: [PATCH 5/6] media: platform: Use str_enable

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] [day] [month] [year] [list]

Message-ID: <d3cf3e65-fe43-4c0b-bfb5-03fdd3c9fc29@xs4all.nl>
Date: Thu, 20 Feb 2025 14:51:23 +0100
From: Hans Verkuil <hverkuil@...all.nl>
To: Tomasz Figa <tfiga@...omium.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Marek Szyprowski <m.szyprowski@...sung.com>,
 Maxim Levitsky <maximlevitsky@...il.com>, Sean Young <sean@...s.org>,
 Olli Salonen <olli.salonen@....fi>, Abylay Ospan <aospan@...zon.com>,
 Jemma Denson <jdenson@...il.com>,
 Patrick Boettcher <patrick.boettcher@...teo.de>,
 Jacopo Mondi <jacopo+renesas@...ndi.org>,
 Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
 Niklas Söderlund <niklas.soderlund+renesas@...natech.se>,
 Tim Harvey <tharvey@...eworks.com>, Andy Walls <awalls@...metrocast.net>,
 Bingbu Cao <bingbu.cao@...el.com>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] media: platform: Use str_enable_disable-like helpers

On 1/15/25 08:47, Tomasz Figa wrote:
> On Wed, Jan 15, 2025 at 4:39 PM Sakari Ailus
> <sakari.ailus@...ux.intel.com> wrote:
>>
>> Hi Laurent,
>>
>> On Tue, Jan 14, 2025 at 10:42:40PM +0200, Laurent Pinchart wrote:
>>> Hi Krzysztof,
>>>
>>> Thank you for the patch.
>>>
>>> On Tue, Jan 14, 2025 at 08:46:21PM +0100, Krzysztof Kozlowski wrote:
>>>> Replace ternary (condition ? "enable" : "disable") syntax with helpers
>>>> from string_choices.h because:
>>>> 1. Simple function call with one argument is easier to read.  Ternary
>>>>    operator has three arguments and with wrapping might lead to quite
>>>>    long code.
>>>
>>> It's more difficult to read for me.
>>
>> I don't have any issue in using the ternary operator either. Using these
>> helpers makes the lines generally 3 characters shorter.
>>
>>>
>>>> 2. Is slightly shorter thus also easier to read.
>>>> 3. It brings uniformity in the text - same string.
>>>> 4. Allows deduping by the linker, which results in a smaller binary
>>>>    file.
>>>
>>> I don't see why the linker can't de-dup string in the current code.
>>
>> In fact the functions are static inline so from that point of view I don't
>> think there's any difference.
>>
>>>
>>> I'm sorry, I just don't see the point in doing this. I'd like to avoid
>>> those changes in the Linux media subsystem, or at the very least in
>>> drivers I maintain.
>>
>> I don't have much of an opinion, perhaps I slightly prefer using these as
>> the rest of the kernel does, too. Yet if we choose not to use these
>> helpers, we continue to be occasional targets of largish patchsets "fixing"
>> this.
> 
> To put one more aspect on the scales:
> 
> These kinds of patches actually make it more difficult to backport
> changes (e.g. fixes) to stable kernels, so my preference would be to
> only use the new helpers in new drivers.

I agree with Tomasz. Now, if the whole kernel is converting to these
new functions, then I guess we should follow, but from what I can tell
that doesn't appear to be the case.

I'll reject this series. It can always be resurrected if there is
sufficient demand for this.

Regards,

	Hans

> 
> Best regards,
> Tomasz
>