diff mbox series

[2/4] linux/array_size.h: Add ARRAY_SIZE_OF_FIELD()

Message ID 20230817143352.132583-3-lucas.segarra.fernandez@intel.com
State New
Headers show
Series Add debugfs pm_status for qat driver | expand

Commit Message

Lucas Segarra Fernandez Aug. 17, 2023, 2:33 p.m. UTC
Introduce ARRAY_SIZE_OF_FIELD() in order to get the number of elements
of an array struct field.

Signed-off-by: Lucas Segarra Fernandez <lucas.segarra.fernandez@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 include/linux/array_size.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Alejandro Colomar Aug. 17, 2023, 8:34 p.m. UTC | #1
Hi Lucas,

On 2023-08-17 16:33, Lucas Segarra Fernandez wrote:
> Introduce ARRAY_SIZE_OF_FIELD() in order to get the number of elements
> of an array struct field.
> 
> Signed-off-by: Lucas Segarra Fernandez <lucas.segarra.fernandez@intel.com>
> Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  include/linux/array_size.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/array_size.h b/include/linux/array_size.h
> index 06d7d83196ca..37dac0473b5c 100644
> --- a/include/linux/array_size.h
> +++ b/include/linux/array_size.h
> @@ -10,4 +10,12 @@
>   */
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
>  
> +/*
> + * ARRAY_SIZE_OF_FIELD - get the number of elements of an array struct field
> + *
> + * @TYPE: The structure containing the field of interest
> + * @MEMBER: The array field to be sized
> + */
> +#define ARRAY_SIZE_OF_FIELD(TYPE, MEMBER) ARRAY_SIZE((((TYPE *)0)->MEMBER))

Some comment about the name:

ARRAY_SIZE() is rather ambiguous, as there's array_size()[1], which means the
number of bytes needed to represent the array.  I suggest a name based on

-  _Lengthof()  It has been proposed to ISO C to get the number of elements
                of an array:
                <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2529.pdf>.

-  sizeof_field()  The kernel macro for the size of a struct member

So, how about lengthof_field()?


Cheers,
Alex


[1]:  The definition of array_size() is here:

$ grep -rnB10 define.array_size include/linux/overflow.h 
238-/**
239- * array_size() - Calculate size of 2-dimensional array.
240- * @a: dimension one
241- * @b: dimension two
242- *
243- * Calculates size of 2-dimensional array: @a * @b.
244- *
245- * Returns: number of bytes needed to represent the array or SIZE_MAX on
246- * overflow.
247- */
248:#define array_size(a, b)	size_mul(a, b)


> +
>  #endif  /* _LINUX_ARRAY_SIZE_H */
Andy Shevchenko Aug. 17, 2023, 9:30 p.m. UTC | #2
On Thu, Aug 17, 2023 at 11:34 PM Alejandro Colomar <alx@kernel.org> wrote:
> On 2023-08-17 16:33, Lucas Segarra Fernandez wrote:

...

> Some comment about the name:
>
> ARRAY_SIZE() is rather ambiguous, as there's array_size()[1], which means the
> number of bytes needed to represent the array.  I suggest a name based on
>
> -  _Lengthof()  It has been proposed to ISO C to get the number of elements
>                 of an array:
>                 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2529.pdf>.
>
> -  sizeof_field()  The kernel macro for the size of a struct member
>
> So, how about lengthof_field()?

TBH I do not understand the motivation of making this kind of
confusion and inconsistency. Are you suggesting renaming ARRAY_SIZE()
to begin with? If so, it's definitely out of the scope of this series.
Alejandro Colomar Aug. 17, 2023, 10:03 p.m. UTC | #3
Hello Andy,

On 2023-08-17 23:30, Andy Shevchenko wrote:
> On Thu, Aug 17, 2023 at 11:34 PM Alejandro Colomar <alx@kernel.org> wrote:
>> On 2023-08-17 16:33, Lucas Segarra Fernandez wrote:
> 
> ...
> 
>> Some comment about the name:
>>
>> ARRAY_SIZE() is rather ambiguous, as there's array_size()[1], which means the
>> number of bytes needed to represent the array.  I suggest a name based on
>>
>> -  _Lengthof()  It has been proposed to ISO C to get the number of elements
>>                 of an array:
>>                 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2529.pdf>.
>>
>> -  sizeof_field()  The kernel macro for the size of a struct member
>>
>> So, how about lengthof_field()?
> 
> TBH I do not understand the motivation of making this kind of
> confusion and inconsistency.


> Are you suggesting renaming ARRAY_SIZE()
> to begin with?

No.  ARRAY_SIZE is a very old and known API.  It is array_size() that is
to be blamed, due to having a confusing name.

What I suggest is not reusing the root of the name of ARRAY_SIZE(), which
since the addition of array_size() may be less unambiguous (IMHO).

So, instead of ARRAY_SIZE_*(), which is derived from ARRAY_SIZE(), maybe
it would be more unambiguous to use the _Lengthof() name as a root, since
nobody has messed with it so far.

My suggestion is to keep ARRAY_SIZE() with its old name, but use
lengthof_field() for this struct variant of it.  It's also a shorter name,
which will make for shorter lines.

b19d57d0f3cc ("overflow.h: Add flex_array_size() helper")
	- (Mon Jun 8 20:22:33 2020 -0500)
610b15c50e86 ("overflow.h: Add allocation size calculation helpers")
	- (Mon May 7 16:47:02 2018 -0700)

Nevertheless, it was just a minor suggestion, and if array_size() was
seen as a good enough name, it wouldn't be as confusing as it seems to
me.  If you find them to be fine, go ahead.

Cheers,
Alex

> If so, it's definitely out of the scope of this series.
>
diff mbox series

Patch

diff --git a/include/linux/array_size.h b/include/linux/array_size.h
index 06d7d83196ca..37dac0473b5c 100644
--- a/include/linux/array_size.h
+++ b/include/linux/array_size.h
@@ -10,4 +10,12 @@ 
  */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
 
+/*
+ * ARRAY_SIZE_OF_FIELD - get the number of elements of an array struct field
+ *
+ * @TYPE: The structure containing the field of interest
+ * @MEMBER: The array field to be sized
+ */
+#define ARRAY_SIZE_OF_FIELD(TYPE, MEMBER) ARRAY_SIZE((((TYPE *)0)->MEMBER))
+
 #endif  /* _LINUX_ARRAY_SIZE_H */