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: <CAAFQd5ASMNr2tO+R1sYa58diWLHJr8iomKA2-PinEojCdXT98A@mail.gmail.com>
Date: Wed, 15 Jan 2025 16:47:52 +0900
From: Tomasz Figa <tfiga@...omium.org>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, Hans Verkuil <hverkuil@...all.nl>, 
	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 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.

Best regards,
Tomasz