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

Message-ID: <Z4dmFlJ_0hugX2rY@kekkonen.localdomain>
Date: Wed, 15 Jan 2025 07:39:02 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
	Hans Verkuil <hverkuil@...all.nl>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Tomasz Figa <tfiga@...omium.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

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.

-- 
Kind regards,

Sakari Ailus