Re: update checkpatch.pl to version 0.19

[Andy Whitcroft <apw@shadowen.org>]

On Sat, Jun 07, 2008 at 10:34:36AM -0700, Greg KH wrote:
> On Fri, Jun 06, 2008 at 11:02:39PM -0700, Andrew Morton wrote:
> > On Fri, 6 Jun 2008 16:30:37 -0700 Greg KH <greg@kroah.com> wrote:
> > 
> > > On Fri, Jun 06, 2008 at 07:22:25PM +0000, Linux Kernel Mailing List wrote:
> > > > @@ -1920,23 +2004,16 @@ sub process {
> > > >  				WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev);
> > > >  			}
> > > >  		}
> > > > -# check for needless usb_free_urb() checks
> > > > -		if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
> > > > -			my $expr = $1;
> > > > -			if ($line =~ /\busb_free_urb\(\Q$expr\E\);/) {
> > > > -				WARN("usb_free_urb(NULL) is safe this check is probabally not required\n" . $hereprev);
> > > > -			}
> > > > -		}
> > > >  
> > > 
> > > I'm curious as to why this check was removed.  Any specific reason?  It
> > > was valid from what I can tell.
> > 
> > Because of the spelling?
> > 
> > If so, let's be consistent:
> > 
> > --- a/scripts/checkpatch.pl~a
> > +++ a/scripts/checkpatch.pl
> > @@ -1997,14 +1997,6 @@ sub process {
> >  				$herecurr);
> >  		}
> >  
> > -# check for needless kfree() checks
> > -		if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
> > -			my $expr = $1;
> > -			if ($line =~ /\bkfree\(\Q$expr\E\);/) {
> > -				WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev);
> > -			}
> > -		}
> > -
> 
> That's the proverbial baby with the bathwater problem here, we can fix
> spelling mistakes pretty easily :)
> 
> I'd prefer the original check to be put back, and the kfree(NULL) check
> to remain as well, as it too is valid.
> 
> But I would like to find out first from Andy why this was removed.  Odds
> are he wasn't keeping up with the changes upstream from his local copy,
> which also might have caused other things to be removed over time :(

That look to be my fault.  Lost something that got merged by some other
route.  I really need a workable solution to the tests problem so I can
work with real per change deltas.

I do try and check things are not removed erroneously.  Seems I did a
poor job on that one tho.  The delta was pretty heafty as I remember, so
I missed it.

The issue I have is I have a comprehensive test suite which is kept in
lock step with checkpatch itself, all in my own tree.  Now that is just
noise for mainline.  What I want to be able to do is keep that in lock
step with developments in my tree, but not merge it upstream.  So far
I've not found a workable solution with git for that.

Anyhow I will sort out where that check has gone and reinstate it.
Perhaps for the next release I will move to pushing Andrew as a set of
patches, not a delta that would stop this occuring again.

-apw