| Message ID | 20200717072319.101302-1-irogers@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | libbpf bpf_helpers: Use __builtin_offsetof for offsetof if available | expand |
On 7/17/20 12:23 AM, Ian Rogers wrote: > The non-builtin route for offsetof has a dependency on size_t from > stdlib.h/stdint.h that is undeclared and may break targets. > The offsetof macro in bpf_helpers may disable the same macro in other > headers that have a #ifdef offsetof guard. Rather than add additional > dependencies improve the offsetof macro declared here to use the > builtin if available. > > Signed-off-by: Ian Rogers <irogers@google.com> Thanks. I didn't pay attention to __builtin_offsetof() and clearly it is preferred. Ack the change with a nit below. Acked-by: Yonghong Song <yhs@fb.com> > --- > tools/lib/bpf/bpf_helpers.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > index a510d8ed716f..ed2ac74fc515 100644 > --- a/tools/lib/bpf/bpf_helpers.h > +++ b/tools/lib/bpf/bpf_helpers.h > @@ -40,8 +40,12 @@ > * Helper macro to manipulate data structures > */ > #ifndef offsetof > +#if __has_builtin(__builtin_offsetof) > +#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER) The __builtin_offsetof is actually available at least since llvm 3.7, so it is safe to use it always. > +#else > #define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER) > #endif > +#endif > #ifndef container_of > #define container_of(ptr, type, member) \ > ({ \ >
On Fri, Jul 17, 2020 at 12:24 AM Ian Rogers <irogers@google.com> wrote: > > The non-builtin route for offsetof has a dependency on size_t from > stdlib.h/stdint.h that is undeclared and may break targets. > The offsetof macro in bpf_helpers may disable the same macro in other > headers that have a #ifdef offsetof guard. Rather than add additional > dependencies improve the offsetof macro declared here to use the > builtin if available. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/lib/bpf/bpf_helpers.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > index a510d8ed716f..ed2ac74fc515 100644 > --- a/tools/lib/bpf/bpf_helpers.h > +++ b/tools/lib/bpf/bpf_helpers.h > @@ -40,8 +40,12 @@ > * Helper macro to manipulate data structures > */ > #ifndef offsetof > +#if __has_builtin(__builtin_offsetof) > +#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER) > +#else > #define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER) Let's either always use __builtin_offsetof (as Yonghong mentioned, it should always be available on relevant LLVM versions). Or instead of size_t, just cast to (unsigned long), I think it will have absolutely the same effect as size_t casting? > #endif > +#endif > #ifndef container_of > #define container_of(ptr, type, member) \ > ({ \ > -- > 2.28.0.rc0.105.gf9edc3c819-goog >
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h index a510d8ed716f..ed2ac74fc515 100644 --- a/tools/lib/bpf/bpf_helpers.h +++ b/tools/lib/bpf/bpf_helpers.h @@ -40,8 +40,12 @@ * Helper macro to manipulate data structures */ #ifndef offsetof +#if __has_builtin(__builtin_offsetof) +#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER) +#else #define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER) #endif +#endif #ifndef container_of #define container_of(ptr, type, member) \ ({ \
The non-builtin route for offsetof has a dependency on size_t from stdlib.h/stdint.h that is undeclared and may break targets. The offsetof macro in bpf_helpers may disable the same macro in other headers that have a #ifdef offsetof guard. Rather than add additional dependencies improve the offsetof macro declared here to use the builtin if available. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/lib/bpf/bpf_helpers.h | 4 ++++ 1 file changed, 4 insertions(+)