linux-kernel - Re: [PATCH v2] <sys/param.h>: Add nitems() and snitems() macros

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: <22c110fe-4c92-e5e6-dc35-dbf00a97cfa2@gmail.com>
Date:   Fri, 25 Sep 2020 18:30:28 +0200
From:   Alejandro Colomar <colomar.6.4.3@...il.com>
To:     Jonathan Wakely <jwakely@...hat.com>
Cc:     libc-alpha@...rceware.org, libc-coord@...ts.openwall.com,
        libstdc++@....gnu.org, gcc@....gnu.org,
        linux-kernel@...r.kernel.org, linux-man@...r.kernel.org,
        fweimer@...hat.com, ville.voutilainen@...il.com, enh@...gle.com,
        rusty@...tcorp.com.au
Subject: Re: [PATCH v2] <sys/param.h>: Add nitems() and snitems() macros

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