linux-kernel - Re: [PATCH v4 04/14] iio: accel: adxl345: introduce adxl345_push

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: <Z9ctSODRTxI53jAY@surfacebook.localdomain>
Date: Sun, 16 Mar 2025 21:58:00 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
	eraretuya@...il.com
Subject: Re: [PATCH v4 04/14] iio: accel: adxl345: introduce
 adxl345_push_event function

Thu, Mar 13, 2025 at 04:50:39PM +0000, Lothar Rubusch kirjoitti:
> Move the fifo handling into a separate function. This is a preparation
> for a generic handling of the interrupt status register results.
> 
> The interrupt status register is read into a variable int_stat. It carries
> status for various sensor events, handling of which is added in follow up
> patches. Evaluation of the int_stat variable is common for sensor events,
> such as tap detection, freefall, activity,... and for fifo events, such as
> data ready, overrun, watermark,... Also, dealing with in case error
> returns shall be common to all events. Thus migrate fifo read-out and push
> fifo content to iio channels into this function to be built up with
> additional event handling.

...

> +static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat)
> +{
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	int samples;
> +	int ret = -ENOENT;
> +
> +	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
> +		samples = adxl345_get_samples(st);
> +		if (samples < 0)

> +			return -EINVAL;

In the original code it makes no difference, but if you are going to share
this, I would expect to see

			return samples;

here. Why the error code is shadowed? If it's trully needed, it has to be
explained in the comment.


> +		if (adxl345_fifo_push(indio_dev, samples) < 0)
> +			return -EINVAL;
> +	}
> +
> +	return ret;
> +}

...

Jonathan, I saw that you had applied it, but I guess the above needs
a clarification.

-- 
With Best Regards,
Andy Shevchenko