[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzbKfqef+GS8UrpXNPZRZ+Nk-w+wjnNFY1SLJYPxYq3DBg@mail.gmail.com>
Date: Mon, 5 Jan 2026 16:46:33 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: WanLi Niu <kiraskyler@....com>
Cc: Quentin Monnet <qmo@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Menglong Dong <menglong8.dong@...il.com>, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, WanLi Niu <niuwl1@...natelecom.cn>,
Menglong Dong <dongml2@...natelecom.cn>
Subject: Re: [PATCH v3 bpf-next] bpftool: Make skeleton C++ compatible with
explicit casts
On Sat, Jan 3, 2026 at 6:15 PM WanLi Niu <kiraskyler@....com> wrote:
>
> From: WanLi Niu <niuwl1@...natelecom.cn>
>
> Fix C++ compilation errors in generated skeleton by adding explicit
> pointer casts and using integer subtraction for offset calculation.
>
> Use struct outer::inner syntax under __cplusplus to access nested skeleton map
> structs, ensuring C++ compilation compatibility while preserving C support
>
> error: invalid conversion from 'void*' to '<obj_name>*' [-fpermissive]
> | skel = skel_alloc(sizeof(*skel));
> | ~~~~~~~~~~^~~~~~~~~~~~~~~
> | |
> | void*
>
> error: arithmetic on pointers to void
> | skel->ctx.sz = (void *)&skel->links - (void *)skel;
> | ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
>
> error: assigning to 'struct <obj_name>__<ident> *' from incompatible type 'void *'
> | skel-><ident> = skel_prep_map_data((void *)data, 4096,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | sizeof(data) - 1);
> | ~~~~~~~~~~~~~~~~~
>
> error: assigning to 'struct <obj_name>__<ident> *' from incompatible type 'void *'
> | skel-><ident> = skel_finalize_map_data(&skel->maps.<ident>.initial_value,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | 4096, PROT_READ | PROT_WRITE, skel->maps.<ident>.map_fd);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: WanLi Niu <niuwl1@...natelecom.cn>
> Co-developed-by: Menglong Dong <dongml2@...natelecom.cn>
> Signed-off-by: Menglong Dong <dongml2@...natelecom.cn>
> ---
> changelog:
> v3:
> - Fix two additional <obj_name>__<ident> type mismatches as suggested by Yonghong Song
>
> v2: https://lore.kernel.org/all/20251231102929.3843-1-kiraskyler@163.com/
> - Use generic (struct %1$s *) instead of project-specific (struct trace_bpf *)
>
> v1: https://lore.kernel.org/all/20251231092541.3352-1-kiraskyler@163.com/
> ---
> tools/bpf/bpftool/gen.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 993c7d9484a4..010861b7d0ea 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -731,10 +731,10 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> { \n\
> struct %1$s *skel; \n\
> \n\
> - skel = skel_alloc(sizeof(*skel)); \n\
> + skel = (struct %1$s *)skel_alloc(sizeof(*skel)); \n\
> if (!skel) \n\
> goto cleanup; \n\
> - skel->ctx.sz = (void *)&skel->links - (void *)skel; \n\
> + skel->ctx.sz = (__u64)&skel->links - (__u64)skel; \n\
I'm wondering if this can also trigger some warnings under some
circumstances? void * is castable to long or unsigned long, but __u64
can be defined as long long (plus it's a question what happens on
32-bit architectures). Why worry about this if we can cast to `char *`
to calculate this size?
> ",
> obj_name, opts.data_sz);
> bpf_object__for_each_map(map, obj) {
> @@ -755,13 +755,17 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> \n\
> \"; \n\
> \n\
> + #ifdef __cplusplus \n\
> + skel->%1$s = (struct %3$s::%3$s__%1$s *)skel_prep_map_data((void *)data, %2$zd,\n\
we already use __typeof__() (see gen_st_ops_shadow_init), so let's use
that unconditionally without __cplusplus special casing
pw-bot: cr
> + #else \n\
> skel->%1$s = skel_prep_map_data((void *)data, %2$zd,\n\
> + #endif \n\
> sizeof(data) - 1);\n\
> if (!skel->%1$s) \n\
> goto cleanup; \n\
> skel->maps.%1$s.initial_value = (__u64) (long) skel->%1$s;\n\
> } \n\
> - ", ident, bpf_map_mmap_sz(map));
> + ", ident, bpf_map_mmap_sz(map), obj_name);
> }
> codegen("\
> \n\
> @@ -857,12 +861,16 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
>
> codegen("\
> \n\
> + #ifdef __cplusplus \n\
> + skel->%1$s = (struct %4$s::%4$s__%1$s *)skel_finalize_map_data(&skel->maps.%1$s.initial_value,\n\
same as above, __typeof__ ?
> + #else \n\
> skel->%1$s = skel_finalize_map_data(&skel->maps.%1$s.initial_value, \n\
> + #endif \n\
> %2$zd, %3$s, skel->maps.%1$s.map_fd);\n\
> if (!skel->%1$s) \n\
> return -ENOMEM; \n\
> ",
> - ident, bpf_map_mmap_sz(map), mmap_flags);
> + ident, bpf_map_mmap_sz(map), mmap_flags, obj_name);
> }
> codegen("\
> \n\
> --
> 2.39.1
>
Powered by blists - more mailing lists