linux-kernel - Re: [PATCH 1/4] iio: introduce IIO_DECLARE_BUFFER_WITH

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: <8b5687ec66236b0869c864f65e2e04fd6e58e725.camel@gmail.com>
Date: Mon, 21 Apr 2025 14:24:36 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Jonathan Cameron
 <jic23@...nel.org>,  Nuno Sá	 <nuno.sa@...log.com>, Andy
 Shevchenko <andy@...nel.org>, Lars-Peter Clausen	 <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] iio: introduce IIO_DECLARE_BUFFER_WITH_TS

On Fri, 2025-04-18 at 17:58 -0500, David Lechner wrote:
> Add a new macro to help with the common case of declaring a buffer that
> is safe to use with iio_push_to_buffers_with_ts(). This is not trivial
> to do correctly because of the alignment requirements of the timestamp.
> This will make it easier for both authors and reviewers.
> 
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
>  include/linux/iio/iio.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index
> 638cf2420fbd85cf2924d09d061df601d1d4bb2a..f523b046c627037c449170b7490ce2a351c6
> b9c0 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -7,6 +7,7 @@
>  #ifndef _INDUSTRIAL_IO_H_
>  #define _INDUSTRIAL_IO_H_
>  
> +#include <linux/align.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
>  #include <linux/compiler_types.h>
> @@ -770,6 +771,21 @@ static inline void *iio_device_get_drvdata(const struct
> iio_dev *indio_dev)
>  	return dev_get_drvdata(&indio_dev->dev);
>  }
>  
> +/**
> + * IIO_DECLARE_BUFFER_WITH_TS() - Declare a buffer with timestamp
> + * @type: element type of the buffer
> + * @name: identifier name of the buffer
> + * @count: number of elements in the buffer
> + *
> + * Declares a buffer that is safe to use with iio_push_to_buffer_with_ts().
> In
> + * addition to allocating enough space for @count elements of @type, it also
> + * allocates space for a s64 timestamp at the end of the buffer and ensures
> + * proper alignment of the timestamp.
> + */
> +#define IIO_DECLARE_BUFFER_WITH_TS(type, name, count) \
> +	type name[ALIGN((count), sizeof(s64) / sizeof(type)) \
> +		  + sizeof(s64)/ sizeof(type)] __aligned(sizeof(s64))
> +

I tend to agree with Andy on the two alignments thing. Independent of that I
guess this is also not intended for stack allocations? If so, we should maybe be
clear about that in the docs... If we do intend using stack allocations we could
improve it by making sure the build fails if "count" is not a constant
expression (likely it will already but I think we should make the intent clear).

- Nuno Sá

>  /*
>   * Used to ensure the iio_priv() structure is aligned to allow that structure
>   * to in turn include IIO_DMA_MINALIGN'd elements such as buffers which