From: Quentin Monnet <quentin@isovalent.com>
To: "Yonghong Song" <yhs@fb.com>,
"Jörn-Thorben Hinz" <jthinz@mailbox.tu-berlin.de>,
bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>,
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>
Subject: Re: [RFC PATCH bpf-next] bpftool: Mark generated skeleton headers as system headers
Date: Fri, 29 Jul 2022 11:12:11 +0100 [thread overview]
Message-ID: <31673eca-ec46-35b2-9172-4156d985b621@isovalent.com> (raw)
In-Reply-To: <7d4af6b4-f4da-f004-48a1-e408d8615ee8@fb.com>
On 28/07/2022 19:25, Yonghong Song wrote:
>
>
> On 7/28/22 9:56 AM, Jörn-Thorben Hinz wrote:
>> Hi,
>>
>> after compiling a skeleton-using program with -pedantic once and
>> stumbling across a tiniest incorrectness in skeletons with it[1], I was
>> debating whether it makes sense to suppress warnings from skeleton
>> headers.
>>
>> Happy about comments about this. This change might be too suppressive
>> towards warnings and maybe ignoring only -Woverlength-strings directly
>> in OBJ_NAME__elf_bytes() be a better idea. Or keep all warnings from
>> skeletons available as-is to have them more visible in and around
>> bpftool’s development.
>
> This is my 2cents. As you mentioned, skeleton file are per program
> and not in system header file directory. I would like not to mark
> these header files as system files. Since different program will
> generate different skeleton headers, suppressing warnings
> will prevent from catching potential issues in certain cases.
>
> Also, since the warning is triggered by extra user flags like -pedantic
> when building bpftool, user can also add -Wno-overlength-strings
> in the extra user flags.
I agree with Yonghong, I don't think it's a good idea to mark the whole
file as a system header. I would maybe consider the other solution where
we can disable the warning locally in the skeleton, just around
OBJ_NAME__elf_bytes() as you suggested. Although I suppose we'd need
several pragmas if we want to silence it for GCC and clang, for example?
It looks like your patch was only addressing GCC?
Thanks for the contribution,
Quentin
next prev parent reply other threads:[~2022-07-29 10:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-28 16:56 [RFC PATCH bpf-next] bpftool: Mark generated skeleton headers as system headers Jörn-Thorben Hinz
2022-07-28 18:25 ` Yonghong Song
2022-07-29 10:12 ` Quentin Monnet [this message]
2022-07-29 17:06 ` Yonghong Song
2022-07-29 17:29 ` Jörn-Thorben Hinz
2022-07-29 17:29 ` Jörn-Thorben Hinz
2022-08-02 23:16 ` Yonghong Song
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=31673eca-ec46-35b2-9172-4156d985b621@isovalent.com \
--to=quentin@isovalent.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=jthinz@mailbox.tu-berlin.de \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yhs@fb.com \
/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