Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)

From: One Thousand Gnomes
Date: Mon Dec 15 2014 - 06:44:58 EST


On Sat, 13 Dec 2014 11:46:47 -0800
Jeremiah Mahler <jmmahler@xxxxxxxxx> wrote:

> LoÃc,
>
> On Sat, Dec 13, 2014 at 07:22:38PM +0100, Loic Pefferkorn wrote:
> > > Whose convention is this? I can't find any mention in
> > > Documention/CodingStyle. checkpatch.pl doesn't complain about them.
> > > And there are almost three thousand examples in staging which don't
> > > use this convention.
> > >
> > > linux-next$ grep -r "== NULL" drivers/staging/* | wc -l
> > > 2844
> >
> > Hi Jeremiah,
> >
> > Thanks for your feedback.
> >
> > I have used checkpatch.pl with the --strict flag:

checkpatch.pl is a bit dubious at the best of times - you can't automate
taste without an AI ;). With --strict it's a positive hazard.

Those kind of small cleanups really only make sense if you are doing big
changes to the code itself anyway and are doing testing and all the rest.

In this case I'd say checkpatch.pl is actually wrong because in the
general case it's better to compare with NULL in C

If you write

if (!x)

and accidentally use a non-pointer type you don't get a warning. If you
try and compare a non pointer type to NULL you usually do. So the NULL
comparison avoids accidents.

The historical reason for it being done in C was I think to avoid the

if (x = NULL)

bug, but gcc will shout at you for that these days.

Alan



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/