diff mbox series

[v2] <sys/param.h>: Add nitems() and snitems() macros

Message ID 20200925132000.235033-1-colomar.6.4.3@gmail.com
State Not Applicable
Headers show
Series [v2] <sys/param.h>: Add nitems() and snitems() macros | expand

Commit Message

Alejandro Colomar Sept. 25, 2020, 1:20 p.m. UTC
'nitems()' calculates the length of an array in number of items.
It is safe: if a pointer is passed to the macro (or function, in C++),
the compilation is broken due to:
 - In >= C11: _Static_assert()
 - In C89, C99: Negative anonymous bitfield
 - In C++: The template requires an array

'snitems()' is equivalent to nitems(),
but it returns a 'ptrdiff_t' instead of a 'size_t'.
It is useful for comparison with signed integer values.

Some BSDs already provide a macro nitems() in <sys/param.h>,
although it usually doesn't provide safety against pointers.

This patch uses the same name for compatibility reasons,
and to be the least disruptive with existing code.

This patch also adds some other macros, which are required by 'nitems()':

__is_same_type(_A, _B):
Returns non-zero if the two input arguments are of the same type.

__is_array(_Arr):
Returns non-zero if the input argument is of an array type.

__must_be(_Expr, _Msg):
Allows using _Static_assert() everywhere an expression can be used.
It evaluates '(int)0' or breaks the compilation.

__must_be_array(_Arr):
It evaluates to '(int)0' if the argument is of an array type.
Else, it breaks compilation.

__array_len(_Arr):
It implements the basic sizeof division needed to calculate the array length.


P.S.: I'd like to put this patch in the public domain.


Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
---
 misc/sys/param.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

Alejandro Colomar Sept. 25, 2020, 2:10 p.m. UTC | #1
On 2020-09-25 15:20, Alejandro Colomar wrote:
 > 'nitems()' calculates the length of an array in number of items.
 > It is safe: if a pointer is passed to the macro (or function, in C++),
 > the compilation is broken due to:
 >   - In >= C11: _Static_assert()
 >   - In C89, C99: Negative anonymous bitfield
 >   - In C++: The template requires an array
 >
 > 'snitems()' is equivalent to nitems(),
 > but it returns a 'ptrdiff_t' instead of a 'size_t'.
 > It is useful for comparison with signed integer values.
 >
 > Some BSDs already provide a macro nitems() in <sys/param.h>,
 > although it usually doesn't provide safety against pointers.
 >
 > This patch uses the same name for compatibility reasons,
 > and to be the least disruptive with existing code.
 >
 > This patch also adds some other macros, which are required by 'nitems()':
 >
 > __is_same_type(_A, _B):
 > Returns non-zero if the two input arguments are of the same type.
 >
 > __is_array(_Arr):
 > Returns non-zero if the input argument is of an array type.
 >
 > __must_be(_Expr, _Msg):
 > Allows using _Static_assert() everywhere an expression can be used.
 > It evaluates '(int)0' or breaks the compilation.
 >
 > __must_be_array(_Arr):
 > It evaluates to '(int)0' if the argument is of an array type.
 > Else, it breaks compilation.
 >
 > __array_len(_Arr):
 > It implements the basic sizeof division needed to calculate the array 
length.
 >
 >
 > P.S.: I'd like to put this patch in the public domain.
 >
 >
 > Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
 > ---

I patched my own system's <sys/param.h> with this,
and while 'nitems()' works fine,
I had to include <stddef.h> in my main.c to be able to use 'snitems()',
because I didn't have 'ptrdiff_t',
eventhough <sys/param.h> already includes <stddef.h>.

I completely ignore the mechanisms behind system headers including
other system headers.

Moreover, I didn't find 'ptrdiff_t' defined in any of my systems headers
I used 'user@debian:/usr/include$ grep -rn ptrdiff_t'.  Does GCC do magic?

What's the problem with that?  How should I fix the patch?

My system:  Debian bullseye/sid; x86-64; gcc 10; libc 2.31-3

Thanks,

Alex


 >   misc/sys/param.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 >   1 file changed, 60 insertions(+)
 >
 > diff --git a/misc/sys/param.h b/misc/sys/param.h
 > index d7c319b157..88e95c2dba 100644
 > --- a/misc/sys/param.h
 > +++ b/misc/sys/param.h
 > @@ -102,5 +102,65 @@
 >   #define MIN(a,b) (((a)<(b))?(a):(b))
 >   #define MAX(a,b) (((a)>(b))?(a):(b))
 >
 > +/* Macros related to the types of variables */
 > +# define __is_same_type(_A, _B) 
__builtin_types_compatible_p(__typeof__(_A), \
 > + 
__typeof__(_B))
 > +# define __is_array(_Arr)	(!__is_same_type((_Arr), &(_Arr)[0]))
 > +
 > +/* Macros for embedding _Static_assert() in expressions */
 > +# if __STDC_VERSION__ >= 201112L
 > +#  define __must_be(_Expr, _Msg)  ( 
          \
 > +        0 * (int)sizeof( 
          \
 > +          struct { 
          \
 > +            _Static_assert((_Expr), _Msg); 
          \
 > +            char _ISO_C_forbids_a_struct_with_no_members; 
          \
 > +          } 
          \
 > +        ) 
          \
 > +)
 > +# else
 > +#  define __must_be(_Expr, _Msg)  ( 
          \
 > +        0 * (int)sizeof( 
          \
 > +          struct { 
          \
 > +            int  : (-!(_Expr)); 
          \
 > +            char _ISO_C_forbids_a_struct_with_no_members; 
          \
 > +          } 
          \
 > +        ) 
          \
 > +)
 > +# endif
 > +
 > +# define __must_be_array(_Arr)	__must_be(__is_array(_Arr), "Must be 
an array!")
 > +
 > +/* Macros for array sizes */
 > +#if defined(__cplusplus)
 > +# if __cplusplus >= 201103L
 > +template<typename _Tp, std::size_t _Len>
 > +  constexpr inline std::size_t
 > +  nitems(const _Tp(&)[_Len]) __THROW
 > +  {
 > +    return _Len;
 > +  }
 > +
 > +template<typename _Tp, std::size_t _Len>
 > +  constexpr inline std::ptrdiff_t
 > +  snitems(const _Tp(&)[_Len]) __THROW
 > +  {
 > +    return _Len;
 > +  }
 > +
 > +# else /* __cplusplus < 201103L */
 > +template<typename _Tp, std::size_t _Len>
 > +  char
 > +  (&__nitems_chararr(const _Tp(&)[_Len]))[_Len];
 > +
 > +#  define nitems(_Arr)          (sizeof(__nitems_chararr(_Arr)))
 > +#  define snitems(_Arr) 
(static_cast<std::ptrdiff_t>(nitems(_Arr)))
 > +# endif /* __cplusplus < 201103L */
 > +
 > +#else /* !defined(__cplusplus) */
 > +# define __array_len(_Arr)      (sizeof(_Arr) / sizeof((_Arr)[0]))
 > +# define nitems(_Arr)           (__array_len(_Arr) + 
__must_be_array(_Arr))
 > +# define snitems(_Arr)          ((ptrdiff_t)nitems(_Arr))
 > +#endif /* !defined(__cplusplus) */
 > +
 >
 >   #endif  /* sys/param.h */
 >
Jonathan Wakely Sept. 25, 2020, 2:48 p.m. UTC | #2
On 25/09/20 16:10 +0200, Alejandro Colomar wrote:
>
>
>On 2020-09-25 15:20, Alejandro Colomar wrote:
>> 'nitems()' calculates the length of an array in number of items.
>> It is safe: if a pointer is passed to the macro (or function, in C++),
>> the compilation is broken due to:
>>   - In >= C11: _Static_assert()
>>   - In C89, C99: Negative anonymous bitfield
>>   - In C++: The template requires an array
>>
>> 'snitems()' is equivalent to nitems(),
>> but it returns a 'ptrdiff_t' instead of a 'size_t'.
>> It is useful for comparison with signed integer values.
>>
>> Some BSDs already provide a macro nitems() in <sys/param.h>,
>> although it usually doesn't provide safety against pointers.
>>
>> This patch uses the same name for compatibility reasons,
>> and to be the least disruptive with existing code.
>>
>> This patch also adds some other macros, which are required by 'nitems()':
>>
>> __is_same_type(_A, _B):
>> Returns non-zero if the two input arguments are of the same type.
>>
>> __is_array(_Arr):
>> Returns non-zero if the input argument is of an array type.
>>
>> __must_be(_Expr, _Msg):
>> Allows using _Static_assert() everywhere an expression can be used.
>> It evaluates '(int)0' or breaks the compilation.
>>
>> __must_be_array(_Arr):
>> It evaluates to '(int)0' if the argument is of an array type.
>> Else, it breaks compilation.
>>
>> __array_len(_Arr):
>> It implements the basic sizeof division needed to calculate the 
>array length.
>>
>>
>> P.S.: I'd like to put this patch in the public domain.
>>
>>
>> Signed-off-by: Alejandro Colomar <colomar.6.4.3@gmail.com>
>> ---
>
>I patched my own system's <sys/param.h> with this,
>and while 'nitems()' works fine,
>I had to include <stddef.h> in my main.c to be able to use 'snitems()',
>because I didn't have 'ptrdiff_t',
>eventhough <sys/param.h> already includes <stddef.h>.
>
>I completely ignore the mechanisms behind system headers including
>other system headers.
>
>Moreover, I didn't find 'ptrdiff_t' defined in any of my systems headers
>I used 'user@debian:/usr/include$ grep -rn ptrdiff_t'.  Does GCC do magic?
>
>What's the problem with that?  How should I fix the patch?

Do you really need to provide snitems?

Users can use (ptrdiff_t)nitems if needed, can't they?

C++ provides std::ssize because there are reasons to want it in
generic contexts when using the function on arbitrary container-like
objects. But for an array size you know that ptrdiff_t is wide enough
to represent the size of any array.

Do you have a use case that requries snitems, or can you assume YAGNI?
Alejandro Colomar Sept. 25, 2020, 4:30 p.m. UTC | #3
Hello Jonathan,

On 2020-09-25 16:48, Jonathan Wakely wrote:
 > Do you really need to provide snitems?
 >
 > Users can use (ptrdiff_t)nitems if needed, can't they?

They can, but that adds casts in the code,
which makes longer lines that are somewhat harder to read.
To avoid that, users may sometimes omit the cast with possible UB.
BTW, I use

IMO, array indices should be declared as 'ptrdiff_t' always,
and not 'size_t'.  More generically, I use unsigned integer types for two
reasons:  bitwise operations, and library functions that require me to 
do so.

I don't intend to force anyone with my opinion, of course,
but if I were to choose a type for 'nitems()', it would be 'ptrdiff_t'.

However, for legacy reasons people will expect that macro to be unsigned,
so I'd have 'nitems()' unsigned, and then a signed version prefixed with 
an 's'.

Some very interesting links about this topic:

Bjarne Stroustrup (and others) about signed and unsigned integers:
https://www.youtube.com/watch?v=Puio5dly9N8&t=12m56s
https://www.youtube.com/watch?v=Puio5dly9N8&t=42m41s

The two links above are two interesting moments of the same video.

I guess that might be the reason they added std::ssize, BTW.

Google's C++ Style Guide about unsigned integers:
https://google.github.io/styleguide/cppguide.html#Integer_Types

And the most voted StackOverflow answer to the question
'What is the correct type for array indexes in C?':
https://stackoverflow.com/a/3174900/6872717

 >
 > C++ provides std::ssize because there are reasons to want it in
 > generic contexts when using the function on arbitrary container-like
 > objects. But for an array size you know that ptrdiff_t is wide enough
 > to represent the size of any array.>
 > Do you have a use case that requries snitems, or can you assume YAGNI?
 >

I have a few use cases:

1)

int	alx_gnuplot_set_style		(struct Alx_Gnuplot *restrict gnuplot,
					 int style, const char *restrict opt)
{

	if (style < 0  ||  style >= ARRAY_SSIZE(styles))
		return	-1;

	if (alx_strlcpys(gnuplot->style, styles[style],
					ARRAY_SIZE(gnuplot->style), NULL))
		return	-1;
	if (opt)
		return	alx_strbcatf(gnuplot->style, NULL, " %s", opt);
	return	0;

}

[https://github.com/alejandro-colomar/libalx/blob/master/src/extra/plot/setup.c]

2) I have many loops that access arrays; I'll just make up an example of
how I normally access arrays:

void foo(ptrdiff_t nmemb)
{
         int arr[nmemb];

         for (ptrdiff_t i = 0; i < ARRAY_SSIZE(arr); i++)
                 arr[i] = i;
}

Grepping through my code,
I have a similar number of ARRAY_SIZE() and ARRAY_SSIZE().
I could have '#define snitems(arr) ((ptrdiff_t)nitems(arr))' in my projects,
but is it really necessary?


Did I convince you? :-)

Thanks,

Alex
Jonathan Wakely Sept. 25, 2020, 5:39 p.m. UTC | #4
On 25/09/20 18:30 +0200, Alejandro Colomar via Libstdc++ wrote:
>Hello Jonathan,
>
>On 2020-09-25 16:48, Jonathan Wakely wrote:
>> Do you really need to provide snitems?
>>
>> Users can use (ptrdiff_t)nitems if needed, can't they?
>
>They can, but that adds casts in the code,
>which makes longer lines that are somewhat harder to read.
>To avoid that, users may sometimes omit the cast with possible UB.
>BTW, I use
>
>IMO, array indices should be declared as 'ptrdiff_t' always,
>and not 'size_t'.  More generically, I use unsigned integer types for two
>reasons:  bitwise operations, and library functions that require me to 
>do so.
>
>I don't intend to force anyone with my opinion, of course,
>but if I were to choose a type for 'nitems()', it would be 'ptrdiff_t'.
>
>However, for legacy reasons people will expect that macro to be unsigned,
>so I'd have 'nitems()' unsigned, and then a signed version prefixed 
>with an 's'.
>
>Some very interesting links about this topic:
>
>Bjarne Stroustrup (and others) about signed and unsigned integers:
>https://www.youtube.com/watch?v=Puio5dly9N8&t=12m56s
>https://www.youtube.com/watch?v=Puio5dly9N8&t=42m41s
>
>The two links above are two interesting moments of the same video.
>
>I guess that might be the reason they added std::ssize, BTW.

Yes, I'm aware of all the rationale. I already said that it makes
sense in C++ where you have generic code. I am not convinced that it's
necessary to add to <sys/param.h> when all it does is a cast from
size_t to ptrdiff_t.
Jonathan Wakely Sept. 25, 2020, 5:42 p.m. UTC | #5
On 25/09/20 18:30 +0200, Alejandro Colomar via Libstdc++ wrote:
>I have a similar number of ARRAY_SIZE() and ARRAY_SSIZE().
>I could have '#define snitems(arr) ((ptrdiff_t)nitems(arr))' in my projects,
>but is it really necessary?

The barrier for adding something to glibc headers should be a LOT
higher than "I could [do it in my own code], but is it really
necessary?"

>Did I convince you? :-)

No.
Alejandro Colomar Sept. 25, 2020, 5:46 p.m. UTC | #6
On 2020-09-25 19:39, Jonathan Wakely wrote:
 > Yes, I'm aware of all the rationale. I already said that it makes
 > sense in C++ where you have generic code. I am not convinced that it's
 > necessary to add to <sys/param.h> when all it does is a cast from
 > size_t to ptrdiff_t.
 >

While I would prefer a signed version, I could live with only 
'nitems()'.  Having all the __must_be_array thing is the most important 
part.

On 2020-09-25 19:42, Jonathan Wakely wrote:
> On 25/09/20 18:30 +0200, Alejandro Colomar via Libstdc++ wrote:
>> I have a similar number of ARRAY_SIZE() and ARRAY_SSIZE().
>> I could have '#define snitems(arr) ((ptrdiff_t)nitems(arr))' in my 
>> projects,
>> but is it really necessary?
> 
> The barrier for adding something to glibc headers should be a LOT
> higher than "I could [do it in my own code], but is it really
> necessary?"
> 
>> Did I convince you? :-)
> 
> No.
> 
> 

Well, you convinced me :)

I'll rewrite the patch, and the problem about <stddef.h> will vanish.

Cheers,

Alex
diff mbox series

Patch

diff --git a/misc/sys/param.h b/misc/sys/param.h
index d7c319b157..88e95c2dba 100644
--- a/misc/sys/param.h
+++ b/misc/sys/param.h
@@ -102,5 +102,65 @@ 
 #define MIN(a,b) (((a)<(b))?(a):(b))
 #define MAX(a,b) (((a)>(b))?(a):(b))
 
+/* Macros related to the types of variables */
+# define __is_same_type(_A, _B)  __builtin_types_compatible_p(__typeof__(_A), \
+                                                              __typeof__(_B))
+# define __is_array(_Arr)	(!__is_same_type((_Arr), &(_Arr)[0]))
+
+/* Macros for embedding _Static_assert() in expressions */
+# if __STDC_VERSION__ >= 201112L
+#  define __must_be(_Expr, _Msg)  (                                           \
+        0 * (int)sizeof(                                                      \
+          struct {                                                            \
+            _Static_assert((_Expr), _Msg);                                    \
+            char _ISO_C_forbids_a_struct_with_no_members;                     \
+          }                                                                   \
+        )                                                                     \
+)
+# else
+#  define __must_be(_Expr, _Msg)  (                                           \
+        0 * (int)sizeof(                                                      \
+          struct {                                                            \
+            int  : (-!(_Expr));                                               \
+            char _ISO_C_forbids_a_struct_with_no_members;                     \
+          }                                                                   \
+        )                                                                     \
+)
+# endif
+
+# define __must_be_array(_Arr)	__must_be(__is_array(_Arr), "Must be an array!")
+
+/* Macros for array sizes */
+#if defined(__cplusplus)
+# if __cplusplus >= 201103L
+template<typename _Tp, std::size_t _Len>
+  constexpr inline std::size_t
+  nitems(const _Tp(&)[_Len]) __THROW
+  {
+    return _Len;
+  }
+
+template<typename _Tp, std::size_t _Len>
+  constexpr inline std::ptrdiff_t
+  snitems(const _Tp(&)[_Len]) __THROW
+  {
+    return _Len;
+  }
+
+# else /* __cplusplus < 201103L */
+template<typename _Tp, std::size_t _Len>
+  char
+  (&__nitems_chararr(const _Tp(&)[_Len]))[_Len];
+
+#  define nitems(_Arr)          (sizeof(__nitems_chararr(_Arr)))
+#  define snitems(_Arr)         (static_cast<std::ptrdiff_t>(nitems(_Arr)))
+# endif /* __cplusplus < 201103L */
+
+#else /* !defined(__cplusplus) */
+# define __array_len(_Arr)      (sizeof(_Arr) / sizeof((_Arr)[0]))
+# define nitems(_Arr)           (__array_len(_Arr) + __must_be_array(_Arr))
+# define snitems(_Arr)          ((ptrdiff_t)nitems(_Arr))
+#endif /* !defined(__cplusplus) */
+
 
 #endif  /* sys/param.h */