On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote:
> On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote:
>> > To address this, we should also suppress the "-Wmissing-prototypes" warning
>> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as
>> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0.
>>
>> Not sure why these need to be suppressed like this at all,
>> can't you just add the prototype somewhere?
>
> BPF kfuncs are intended for use within BPF programs, and they should
> not be called from other parts of the kernel. Consequently, it is not
> appropriate to include their prototypes in a kernel header file.
How does the caller in the BPF program get the prototype?
>> > @@ -131,14 +131,14 @@
>> > #define __diag_str(s) __diag_str1(s)
>> > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
>> >
>> > -#if GCC_VERSION >= 80000
>> > -#define __diag_GCC_8(s) __diag(s)
>> > +#if GCC_VERSION >= 50100
>> > +#define __diag_GCC_5(s) __diag(s)
>> > #else
>> > -#define __diag_GCC_8(s)
>> > +#define __diag_GCC_5(s)
>> > #endif
>> >
>>
>> This breaks all uses of __diag_ignore that specify
>> version 8 directly. Just add the macros for each version
>> from 5 to 14 here.
>
> It seems that __diag_GCC_8() or __diag_GCC() are not directly used
> anywhere in the kernel, right?
I see three instances:
drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init",
include/linux/compat.h: __diag_ignore(GCC, 8, "-Wattribute-alias", include/linux/syscalls.h: __diag_ignore(GCC, 8, "-Wattribute-alias",
The override-init one should probably use version 5 as well,
but I think the -Wattribute-alias ones require GCC 8 and otherwise
cause a warning about an unknown warning option.
__diag_ignore_all() would also be wrong for the override-init
because the option has a different name in clang
(-Winitializer-overrides).
> Therefore it won't break anything if we just replace __diag_GCC_8()
> with __diag_GCC_5().
> It may be cumbersome to add the macrocs for every GCC version if they
> aren't actively used.
For the _all variant, I would prefer to completely remove
the version logic and just use __diag() directly. I think the
entire point of this is that it is used on all supported
versions.
Arnd
