Re: [PATCH v4 3/4] lib/find_bit: optimize find_next_bit() functions

[Linus Torvalds]

On Mon, Sep 19, 2022 at 6:46 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> > +#define FIND_NEXT_BIT(FETCH, MUNGE, size, start)                             \
> > +({                                                                           \
[..]
> > +out:                                                                         \
>
> I dunno if GCC expression limits the scope of goto labels

No. Labels are function-global by default. If you need block-scope for
them, you need to declare them explicitly in tha block before use with
"__label__".

> but on the safe side you can add a prefix to it, so it becomes:
>
> FIND_NEXT_BIT_out:

That doesn't really help, since if you were to use the macro twice,
you'd still get a name clash.

That said, I'm not convinced any of this matters, since these macros
aren't supposed to be used anywhere else, and in fact, they aren't
even in any header file that would allow anybody else to use them.

So I think all the normal "make macros safe" rules are simply
irrelevant for these cases - despite the readable name, these macros
are local special cases for code generation and avoiding duplication,
not generic "use this macro to find a bit".

So it's one thing if a macro is in a header file to be used by random
code. It's a different thing entirely if it's a specialized local
macro for a local issue, that nobody else is ever going to even see.

So I don't think it would be wrong to use __label__ to block-scope it,
or to use a longer name, but I also don't think it's really required.

It's not exactly super-common, but we have various cases of macros
that generate full (or partial) function bodies in various places,
where the macro does various things that should *never* be done in a
"regular" macro that gets used by normal code.

You can see one class of this with something like

   git grep '^static.*##.*(.*\\$' -- '*.c'

but to *really* go blind, see the "SYSCALL_DEFINE*()" macros in
<linux/syscalls.h>.

Those will mess with your mind, and go "whoever wrote those macros
needs to be institutionalized". They do some impressive things,
including creating local functions _and_ starting a new function
definition where the actual code then isn't part of the macro, but
actually just continues where the macro was used.

Which is all very natural and actually looks quite nice and readable
in the places that use it, with users looking like

  SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags)
  {
        int fd;
        struct pid *p;
   ...

which is all pretty legible. But there's no question that that macro
violates every single "normal" macro rule.

The FIND_NEXT_BIT() macro in comparison is pretty tame.

                  Linus