linux-kernel - Re: [PATCH v5 6/8] iio: light: stk3310: use dev_err

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: <54gp7mhpu5fgeeif5amajmcit4znuil2kikx43bcu6eujmq5wu@oxaq6fgmkuyr>
Date: Sat, 15 Feb 2025 15:16:00 -0500
From: Aren <aren@...cevolution.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Jonathan Cameron <jic23@...nel.org>, 
	Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Samuel Holland <samuel@...lland.org>, 
	Kaustabh Chakraborty <kauschluss@...root.org>, Barnabás Czémán <trabarni@...il.com>, 
	Julien Stephan <jstephan@...libre.com>, 
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-sunxi@...ts.linux.dev, Ondrej Jirman <megi@....cz>, Dragan Simic <dsimic@...jaro.org>, 
	phone-devel@...r.kernel.org
Subject: Re: [PATCH v5 6/8] iio: light: stk3310: use dev_err_probe where
 possible

On Sun, Feb 09, 2025 at 04:47:44PM +0200, Andy Shevchenko wrote:
> On Sat, Feb 08, 2025 at 04:13:24PM -0500, Aren Moynihan wrote:
> > Using dev_err_probe instead of dev_err and return makes the errors
> 
> Use dev_err_probe()
> dev_err()
> 
> > easier to understand by including the error name, and saves a little
> > code.
> 
> I believe this patch will make more sense before switching to local 'dev'
> variable. Then the previous one will have an additional justification as
> the "struct device *dev = ...;" lines in some cases will be added already
> by this patch.

That will only be added in one spot, and I skipped updating the dev_err
calls in the previous patch that this patch rewrites, so churn shouldn't
be an issue. That also makes it trivial to reorder them, so I guess it
can't hurt.

> >  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > -	if (!indio_dev) {
> > -		dev_err(&client->dev, "iio allocation failed!\n");
> > -		return -ENOMEM;
> > -	}
> > +	if (!indio_dev)
> > +		return dev_err_probe(dev, -ENOMEM, "iio allocation failed!\n");
> 
> We don't issue the messages for -ENOMEM.
> 
> If it's in the current code, add a new patch to drop this message and return an
> error code directly.
> 
> ...
> 
> > +	if (ret < 0)
> 
> Perhaps, while at it, drop these ' < 0' parts where they are not hinting about
> anything.

Sure, I can add patches for these, although continuing to rebase this
series is getting a bit cumbersome (perhaps just because I haven't found
a good workflow for it). Would I be better off reordering this so the
refactoring patches come first and can be partially merged?

Regards
 - Aren