mbox series

[v2,bpf-next,0/4] Generate NULL in vmlinux.h

Message ID 20210317031257.846314-1-andrii@kernel.org
Headers show
Series Generate NULL in vmlinux.h | expand

Message

Andrii Nakryiko March 17, 2021, 3:12 a.m. UTC 
Generate NULL definition as part of vmlinux.h. This is a pretty common and
unfortunate annoyance that most users of vmlinux.h have to deal with. Patch #2
drops such custom NULL definition in one of the selftests. Patches #3 and #4
make bpftool and selftests compilations stricter by treating warnings as
errors.

v1->v2:
  - fix few typos and wrong copy/paste;
  - fix #pragma push -> pop.

Andrii Nakryiko (4):
  bpftool: generate NULL definition in vmlinux.h
  selftests/bpf: drop custom NULL #define in skb_pkt_end selftest
  selftests/bpf: treat compilation warnings as errors
  bpftool: treat compilation warnings as errors

 tools/bpf/bpftool/Makefile                      | 3 ++-
 tools/bpf/bpftool/btf.c                         | 2 ++
 tools/bpf/bpftool/jit_disasm.c                  | 3 +++
 tools/testing/selftests/bpf/Makefile            | 4 ++--
 tools/testing/selftests/bpf/progs/skb_pkt_end.c | 1 -
 5 files changed, 9 insertions(+), 4 deletions(-)

Comments

Andrii Nakryiko March 17, 2021, 5:36 p.m. UTC | #1 
On Tue, Mar 16, 2021 at 8:13 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Given that vmlinux.h is not compatible with headers like stdint.h, NULL poses
> an annoying problem: it is defined as #define, so is not captured in BTF, so
> is not emitted into vmlinux.h. This leads to users either sticking to explicit
> 0, or defining their own NULL (as progs/skb_pkt_end.c does).
>
> It's pretty trivial for bpftool to generate NULL definition, though, so let's
> just do that. This might cause compilation warning for existing BPF
> applications:
>
>   progs/skb_pkt_end.c:7:9: warning: 'NULL' macro redefined [-Wmacro-redefined]
>   #define NULL 0
>           ^
>   /tmp/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h:4:9: note: previous definition is here
>   #define NULL ((void *)0)
>           ^
>
> It is trivial to fix, though, so long-term benefits outweight temporary
> inconveniences.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/bpf/bpftool/btf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 62953bbf68b4..ff6a76632873 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -405,6 +405,8 @@ static int dump_btf_c(const struct btf *btf,
>         printf("#ifndef __VMLINUX_H__\n");
>         printf("#define __VMLINUX_H__\n");
>         printf("\n");
> +       printf("#define NULL ((void *)0)\n");


On second thought, this could also be done in bpf_helpers.h, which is
pretty much always included in BPF programs. I think that's a bit more
maintainable and less magical to users, so I'll go with that in v3.

> +       printf("\n");
>         printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n");
>         printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n");
>         printf("#endif\n\n");
> --
> 2.30.2
>