Re: [PATCH bpf-next v1 0/6] Change bpftool, libbpf, selftests to force GNU89 mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 8, 2021 at 2:15 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Sat, Nov 6, 2021 at 4:20 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Sat, Nov 6, 2021 at 1:02 PM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> > >
> > > On Sat, Nov 6, 2021 at 9:34 AM Alexei Starovoitov
> > > <alexei.starovoitov@xxxxxxxxx> wrote:
> > > >
> > > > On Fri, Nov 5, 2021 at 6:36 PM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote:
> > > > >
> > > > > Fix any remaining instances that fail the build in this mode.  For selftests, we
> > > > > also need to separate CXXFLAGS from CFLAGS, since adding it to CFLAGS simply
> > > > > would generate a warning when used with g++.
> > > > >
> > > > > This also cherry-picks Andrii's patch to fix the instance in libbpf. Also tested
> > > > > introducing new invalid usage of C99 features.
> > > > >
> > > > > Andrii Nakryiko (1):
> > > > >   libbpf: fix non-C89 loop variable declaration in gen_loader.c
> > > > >
> > > > > Kumar Kartikeya Dwivedi (5):
> > > > >   bpftool: Compile using -std=gnu89
> > > > >   libbpf: Compile using -std=gnu89
> > > > >   selftests/bpf: Fix non-C89 loop variable declaration instances
> > > > >   selftests/bpf: Switch to non-unicode character in output
> > > > >   selftests/bpf: Compile using -std=gnu89
> > > >
> > > > Please don't.
> > > > I'd rather go the other way and drop gnu89 from everywhere.
> > > > for (int i = 0
> > > > is so much cleaner.
> > >
> > > I agree that for (int i) is better, but it's kernel code style which
> > > we followed so far pretty closely for libbpf and bpftool. So I think
> > > this is the right move for bpftool and libbpf.
> >
> > The kernel coding style is not white and black.
> > Certain style preferences are archaic to say the least.
> > It's not the right move to follow it blindly.
>
> Can we at least add -std=gnu89 for the libbpf? It's a library, so
> being conservative with compiler versions and language features makes
> sense there. I'll add a similar flag to Github's Makefile. I'd rather
> catch this at patch submission time rather than at the Github sync
> time.

Sure. Applied Kumar's patch 3.
With CO-RE in the kernel the pieces of libbpf will be part
of the kernel for real, so for libbpf as a whole would make sense
to conform to the language standards as parts of libbpf have to do.
As far as other parts of kernel git the language standard
can be decided whichever way.
perf and libsubcmd (part of objtool) have no issue using 'for (int'
while being part of the kernel tree.
We can adopt strong gnu89 in bpftool, but I'd rather not rush
such a decision right now.
selftests are certainly not gnu89.
All bpf programs are written in C-2021 "standard".
Lots of C extensions in there.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux