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: <20200925144822.GM6061@redhat.com>
Date:   Fri, 25 Sep 2020 15:48:22 +0100
From:   Jonathan Wakely <jwakely@...hat.com>
To:     Alejandro Colomar <colomar.6.4.3@...il.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

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@...il.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@...ian:/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?