On Fri, Sep 4, 2020 at 4:30 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
>
> The extern symbol declaration should be on the BTF_SET_START macro, not
> on BTF_SET_START_GLOBAL, since in the global case the symbol will be
> declared in a header somewhere.
See below about my confusion. But besides that, is there any problem
to have this extern in both BTF_SET_START and BTF_SET_START_GLOBAL?
Are there any problems caused by this? This commit message doesn't
explain what problem it's trying to solve.
>
> Fixes: eae2e83e6263 ("bpf: Add BTF_SET_START/END macros")
> Signed-off-by: Lorenz Bauer <lmb@xxxxxxxxxxxxxx>
> ---
> include/linux/btf_ids.h | 6 +++---
> tools/include/linux/btf_ids.h | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index 210b086188a3..42aa667d4433 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -121,7 +121,8 @@ asm( \
>
> #define BTF_SET_START(name) \
> __BTF_ID_LIST(name, local) \
> -__BTF_SET_START(name, local)
> +__BTF_SET_START(name, local) \
> +extern struct btf_id_set name;
>
> #define BTF_SET_START_GLOBAL(name) \
> __BTF_ID_LIST(name, globl) \
> @@ -131,8 +132,7 @@ __BTF_SET_START(name, globl)
> asm( \
> ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> ".size __BTF_ID__set__" #name ", .-" #name " \n" \
> -".popsection; \n"); \
> -extern struct btf_id_set name;
> +".popsection; \n");
>
> #else
>
> diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
> index 210b086188a3..42aa667d4433 100644
> --- a/tools/include/linux/btf_ids.h
> +++ b/tools/include/linux/btf_ids.h
> @@ -121,7 +121,8 @@ asm( \
>
> #define BTF_SET_START(name) \
> __BTF_ID_LIST(name, local) \
> -__BTF_SET_START(name, local)
> +__BTF_SET_START(name, local) \
> +extern struct btf_id_set name;
>
> #define BTF_SET_START_GLOBAL(name) \
> __BTF_ID_LIST(name, globl) \
> @@ -131,8 +132,7 @@ __BTF_SET_START(name, globl)
> asm( \
> ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> ".size __BTF_ID__set__" #name ", .-" #name " \n" \
> -".popsection; \n"); \
> -extern struct btf_id_set name;
> +".popsection; \n");
>
This diff is extremely misleading. It's actually BTF_SET_END macro.
Coupled with your commit message, it's double-misleading, because you
are moving extern declaration from BTF_SET_END (which is used with
both BTF_SET_START and BTF_SET_START_GLOBAL) to BTF_SET_START. Not
from BTF_SET_START_GLOBAL to BTF_SET_START (as your commit message
implies, at least that's how I read it).
> #else
>
> --
> 2.25.1
>
