On Mon, Mar 11, 2024 at 11:54 AM Ian Rogers <irogers@xxxxxxxxxx> wrote: > > On Mon, Mar 11, 2024 at 10:49 AM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Sat, Mar 9, 2024 at 6:05 PM Ian Rogers <irogers@xxxxxxxxxx> wrote: > > > > > > libbpf depends upon linux/err.h which has a linux/compiler.h > > > dependency. In the kernel includes, as opposed to the tools version, > > > linux/compiler.h includes linux/compiler_attributes.h which defines > > > __printf. As the libbpf.c __printf definition isn't guarded by an > > > ifndef, this leads to a duplicate definition compilation error when > > > trying to update the tools/include/linux/compiler.h. Fix this by > > > adding the missing ifndef. > > > > > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> > > > --- > > > tools/lib/bpf/libbpf.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > > index afd09571c482..2152360b4b18 100644 > > > --- a/tools/lib/bpf/libbpf.c > > > +++ b/tools/lib/bpf/libbpf.c > > > @@ -66,7 +66,9 @@ > > > */ > > > #pragma GCC diagnostic ignored "-Wformat-nonliteral" > > > > > > -#define __printf(a, b) __attribute__((format(printf, a, b))) > > > +#ifndef __printf > > > +# define __printf(a, b) __attribute__((format(printf, a, b))) > > > > styling nit: don't add spaces between # and define, please > > > > overall LGTM > > > > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > > > Two questions, though. > > > > 1. It seems like just dropping #define __printf in libbpf.c compiles > > fine (I checked both building libbpf directly, and BPF selftest, and > > perf, and bpftool directly, all of them built fine). So we can > > probably just drop this. I'll need to add __printf on Github, but > > that's fine. > > > > 2. Logistics. Which tree should this patch go through? Can I land it > > in bpf-next or it's too much inconvenience for you? > > Thanks Andrii, > > dropping the #define (1) sgtm but the current compiler.h will fail to > build libbpf.c without the later compiler.h update in this series. > This causes another logistic issue for your point 2. Presumably if > this patch goes through bpf-next, the first patch "tools bpf: > Synchronize bpf.h with kernel uapi version" should also go through the > bpf-next. > That's what I'm saying, it seems to work without your patches already. At least on bpf-next/master. But it's ok, let's keep it and just add #ifndef guard, that will make my life easier when syncing to Github later one. Then the patch can go through other trees and eventually make it into bpf-next and then Github. So please keep my ack for #ifndef version, thanks. > Thanks, > Ian > > > > > +#endif > > > > > > static struct bpf_map *bpf_object__add_map(struct bpf_object *obj); > > > static bool prog_is_subprog(const struct bpf_object *obj, const struct bpf_program *prog); > > > -- > > > 2.44.0.278.ge034bb2e1d-goog > > >
