From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Ian Rogers <irogers@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
Andrii Nakryiko <andrii@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
Jiri Olsa <jolsa@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Shuah Khan <shuah@kernel.org>, Kees Cook <keescook@chromium.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Bill Wendling <morbo@google.com>,
Justin Stitt <justinstitt@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Liam Howlett <liam.howlett@oracle.com>,
Miguel Ojeda <ojeda@kernel.org>, Will Deacon <will@kernel.org>,
Mark Brown <broonie@kernel.org>,
David Laight <David.Laight@aculab.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Shunsuke Mie <mie@igel.co.jp>,
Yafang Shao <laoar.shao@gmail.com>,
Kui-Feng Lee <kuifeng@meta.com>,
James Clark <james.clark@arm.com>,
Nick Forrington <nick.forrington@arm.com>,
Leo Yan <leo.yan@linux.dev>, German Gomez <german.gomez@arm.com>,
Rob Herring <robh@kernel.org>,
John Garry <john.g.garry@oracle.com>,
Sean Christopherson <seanjc@google.com>,
Anup Patel <anup@brainfault.org>, Fuad Tabba <tabba@google.com>,
Andrew Jones <ajones@ventanamicro.com>,
Chao Peng <chao.p.peng@linux.intel.com>,
Haibo Xu <haibo1.xu@intel.com>, Peter Xu <peterx@redhat.com>,
Vishal Annapurve <vannapurve@google.com>,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
bpf@vger.kernel.org, linux-perf-users@vger.kernel.org,
kvm@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-hardening@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH v1 02/13] libbpf: Make __printf define conditional
Date: Mon, 11 Mar 2024 13:51:49 -0700 [thread overview]
Message-ID: <CAEf4BzZ+4WAaySJVGArk3epJ-u92ULkcoXTr2HnRXshGx8fPDw@mail.gmail.com> (raw)
In-Reply-To: <CAP-5=fUQY=ho1OSk-wosw8=7Sjp8MB_kngggP00BXs+nVNj7Pg@mail.gmail.com>
On Mon, Mar 11, 2024 at 11:54 AM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Mar 11, 2024 at 10:49 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Mar 9, 2024 at 6:05 PM Ian Rogers <irogers@google.com> 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@google.com>
> > > ---
> > > 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@kernel.org>
> >
> > 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
> > >
next prev parent reply other threads:[~2024-03-11 20:52 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-10 2:04 [PATCH v1 00/13] tools header compiler.h update Ian Rogers
2024-03-10 2:04 ` [PATCH v1 01/13] tools bpf: Synchronize bpf.h with kernel uapi version Ian Rogers
2024-03-10 2:04 ` [PATCH v1 02/13] libbpf: Make __printf define conditional Ian Rogers
2024-03-11 17:49 ` Andrii Nakryiko
2024-03-11 18:54 ` Ian Rogers
2024-03-11 20:51 ` Andrii Nakryiko [this message]
2024-03-10 2:04 ` [PATCH v1 03/13] libperf xyarray: Use correct stddef.h include Ian Rogers
2024-03-10 2:04 ` [PATCH v1 04/13] perf expr: Add missing stdbool.h include Ian Rogers
2024-03-10 2:05 ` [PATCH v1 05/13] perf expr: Tidy up header guard Ian Rogers
2024-03-10 2:05 ` [PATCH v1 06/13] perf debug: Add missing linux/types.h include Ian Rogers
2024-03-10 2:05 ` [PATCH v1 07/13] perf cacheline: " Ian Rogers
2024-03-10 2:05 ` [PATCH v1 08/13] perf arm-spe: " Ian Rogers
2024-03-10 2:05 ` [PATCH v1 09/13] tools headers: Rewrite linux/atomic.h using C11's stdatomic.h Ian Rogers
2024-03-10 2:05 ` [PATCH v1 10/13] asm-generic: Avoid transitive dependency for unaligned.h Ian Rogers
2024-03-10 2:05 ` [PATCH v1 11/13] tools headers: Sync linux/overflow.h Ian Rogers
2024-03-10 2:05 ` [PATCH v1 12/13] tools headers: Sync compiler.h headers Ian Rogers
2024-03-10 11:45 ` Miguel Ojeda
2024-03-11 16:34 ` James Clark
2024-03-11 19:24 ` Ian Rogers
2024-03-11 19:58 ` Arnaldo Carvalho de Melo
2024-03-10 2:05 ` [PATCH v1 13/13] tools headers: Rename noinline to __noinline Ian Rogers
2024-03-10 11:24 ` Miguel Ojeda
2024-03-11 15:33 ` Nick Desaulniers
2024-03-11 15:43 ` Michael S. Tsirkin
2024-03-11 16:21 ` Nick Desaulniers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEf4BzZ+4WAaySJVGArk3epJ-u92ULkcoXTr2HnRXshGx8fPDw@mail.gmail.com \
--to=andrii.nakryiko@gmail.com \
--cc=David.Laight@aculab.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ajones@ventanamicro.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=andrii@kernel.org \
--cc=anup@brainfault.org \
--cc=arnd@arndb.de \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=broonie@kernel.org \
--cc=chao.p.peng@linux.intel.com \
--cc=daniel@iogearbox.net \
--cc=german.gomez@arm.com \
--cc=gustavoars@kernel.org \
--cc=haibo1.xu@intel.com \
--cc=haoluo@google.com \
--cc=irogers@google.com \
--cc=james.clark@arm.com \
--cc=john.fastabend@gmail.com \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=justinstitt@google.com \
--cc=keescook@chromium.org \
--cc=kpsingh@kernel.org \
--cc=kuifeng@meta.com \
--cc=kvm@vger.kernel.org \
--cc=laoar.shao@gmail.com \
--cc=leo.yan@linux.dev \
--cc=liam.howlett@oracle.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=mark.rutland@arm.com \
--cc=martin.lau@linux.dev \
--cc=mie@igel.co.jp \
--cc=mingo@redhat.com \
--cc=morbo@google.com \
--cc=mst@redhat.com \
--cc=namhyung@kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=nick.forrington@arm.com \
--cc=ojeda@kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=peterz@infradead.org \
--cc=robh@kernel.org \
--cc=sdf@google.com \
--cc=seanjc@google.com \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=tabba@google.com \
--cc=vannapurve@google.com \
--cc=will@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).