RE: [PATCH] selftests/bpf: use !E instead of comparing with NULL

[Tim.Bird]

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
> 
> On Tue, Apr 13, 2021 at 9:10 AM <Tim.Bird@xxxxxxxx> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
> > >
> > > On Tue, Apr 13, 2021 at 2:52 AM Yang Li <yang.lee@xxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > Fix the following coccicheck warnings:
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:189:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:361:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:386:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:402:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:433:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:534:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:625:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:767:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > >
> > > > Reported-by: Abaci Robot <abaci@xxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Yang Li <yang.lee@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > >  tools/testing/selftests/bpf/progs/profiler.inc.h | 22 +++++++++++-----------
> > > >  1 file changed, 11 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > index 4896fdf8..a33066c 100644
> > > > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > @@ -189,7 +189,7 @@ static INLINE void populate_ancestors(struct task_struct* task,
> > > >  #endif
> > > >         for (num_ancestors = 0; num_ancestors < MAX_ANCESTORS; num_ancestors++) {
> > > >                 parent = BPF_CORE_READ(parent, real_parent);
> > > > -               if (parent == NULL)
> > > > +               if (!parent)
> > >
> > > Sorry, but I'd like the progs to stay as close as possible to the way
> > > they were written.
> > Why?
> >
> > > They might not adhere to kernel coding style in some cases.
> > > The code could be grossly inefficient and even buggy.
> > There would have to be a really good reason to accept
> > grossly inefficient and even buggy code into the kernel.
> >
> > Can you please explain what that reason is?
> 
> It's not the kernel. It's a test of bpf program.
That doesn't answer the question of why you don't want any changes.

Why would we not use kernel coding style guidelines and quality thresholds for
testing code?  This *is* going into the kernel source tree, where it will be
maintained and used by other developers.
 -- Tim