[PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[Andy Shevchenko]

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().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/args.h   | 13 +++++++++++++
 include/linux/kernel.h |  8 +-------
 2 files changed, 14 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/args.h

diff --git a/include/linux/args.h b/include/linux/args.h
new file mode 100644
index 000000000000..16ef6fad8add
--- /dev/null
+++ b/include/linux/args.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_ARGS_H
+#define _LINUX_ARGS_H
+
+/* This counts to 12. Any more, it will return 13th argument. */
+#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
+#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __CONCAT(a, b) a ## b
+#define CONCATENATE(a, b) __CONCAT(a, b)
+
+#endif	/* _LINUX_ARGS_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 0d91e0af0125..fa675d50d7b7 100644
--- 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>
 #include <linux/limits.h>
 #include <linux/linkage.h>
 #include <linux/stddef.h>
@@ -457,13 +458,6 @@ ftrace_vprintk(const char *fmt, va_list ap)
 static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #endif /* CONFIG_TRACING */
 
-/* This counts to 12. Any more, it will return 13th argument. */
-#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
-#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
-
-#define __CONCAT(a, b) a ## b
-#define CONCATENATE(a, b) __CONCAT(a, b)
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[Andrew Morton]

On Tue, 11 Apr 2023 13:24:54 +0300 Andy Shevchenko <andriy.shevchenko@linux.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.

Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[Andy Shevchenko]

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@linux.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 and a couple of users that reimplement this macro
under different names.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[Andrew Morton]

On Wed, 12 Apr 2023 15:56:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.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@linux.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?

> and a couple of users that reimplement this macro
> under different names.

Where are these?

What the heck does it do and why is it so ugly and why isn't it
documented.  Shudder.

I suppose if there are other callers(?) then we could hide it in a
countargs.h and not include that into kernel.h.

Re: [PATCH v1 1/1] kernel.h: Split out COUNT_ARGS() and CONCATENATE()

[Rasmus Villemoes]

On 12/04/2023 20.55, Andrew Morton wrote:
> On Wed, 12 Apr 2023 15:56:43 +0300 Andy Shevchenko <andriy.shevchenko@linux.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@linux.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