linux-kernel - Re: [PATCH v1 1/1] kernel.h: Split out COUNT

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] [day] [month] [year] [list]

Message-ID: <31abaa2d-f93b-e2df-4605-04c276ea7397@rasmusvillemoes.dk>
Date:   Wed, 12 Apr 2023 23:09:55 +0200
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

On 12/04/2023 20.55, Andrew Morton wrote:
> On Wed, 12 Apr 2023 15:56:43 +0300 Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> 
>> On Tue, Apr 11, 2023 at 03:21:19PM -0700, Andrew Morton wrote:
>>> On Tue, 11 Apr 2023 13:24:54 +0300 Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
>>>
>>>> kernel.h is being used as a dump for all kinds of stuff for a long time.
>>>> The COUNT_ARGS() and CONCATENATE() macros may be used in some places
>>>> without need of the full kernel.h dependency train with it.
>>>>
>>>> Here is the attempt on cleaning it up by splitting out these macros().
>>>>
>>>> --- a/include/linux/kernel.h
>>>> +++ b/include/linux/kernel.h
>>>> @@ -13,6 +13,7 @@
>>>>  
>>>>  #include <linux/stdarg.h>
>>>>  #include <linux/align.h>
>>>> +#include <linux/args.h>
>>>
>>> A more energetic patch would have included args.h into each file which
>>> calls COUNT_ARGS() and CONCATENATE(), and not included args.h into
>>> kernel.h.  And that appears to be very easy - only bpf uses these things?
>>>
>>> In fact these macros are so weird and ugly I'd be inclined to move them
>>> into some bpf header so we don't have to see them again.  No
>>> args.h, which might avoid encouraging others to use them.
>>
>> We have more users than one 
> 
> I cant find any?

True, git grep COUNT_ARGS doesn't find anything other than the
bpf_probe.h user.

>> and a couple of users that reimplement this macro
>> under different names.
>>
> Where are these?

Amusingly, bpf have at least

tools/lib/bpf/bpf_core_read.h:#define ___narg(...) ___nth(_,
##__VA_ARGS__, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
tools/lib/bpf/bpf_helpers.h:    ___bpf_nth(_, ##__VA_ARGS__, 12, 11, 10,
9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
tools/lib/bpf/bpf_tracing.h:#define ___bpf_narg(...) ___bpf_nth(_,
##__VA_ARGS__, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

OK, so that's under tools/, but we really should be able to make a
cpp-tricks-header that's usable by the kernel itself as well as tools.

There's also

include/linux/arm-smccc.h:      ___count_args(__VA_ARGS__, 7, 6, 5, 4,
3, 2, 1, 0)
arch/x86/include/asm/rmwcc.h:#define RMWcc_ARGS(X...) __RMWcc_ARGS(,
##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)

and some efi_nargs that seems a little more elaborate (though I doubt
they couldn't just make do with the standard macro).

There's probably even more, this is just what grepping for a typical
implementation showed.

> What the heck does it do

It counts the number of arguments given to a variadic macro.

> and why is it so ugly

Because cpp.

> and why isn't it documented.  Shudder.

Yeah, some comments on how it works and its limitation(s) would probably
be in order.

Rasmus