| Message ID | 20210317031257.846314-2-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Generate NULL in vmlinux.h | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 11 maintainers not CCed: yhs@fb.com jagdsh.linux@gmail.com kpsingh@kernel.org kafai@fb.com iii@linux.ibm.com ast@kernel.org tklauser@distanz.ch john.fastabend@gmail.com tianjia.zhang@linux.alibaba.com songliubraving@fb.com quentin@isovalent.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
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 >
On Wed, Mar 17, 2021 at 10:38 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > 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. yep. good idea.
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"); + 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");
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(+)