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

[One Thousand Gnomes]

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/