lkml.org 
[lkml]   [2008]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: update checkpatch.pl to version 0.19
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


\
 
 \ /
  Last update: 2008-06-09 10:35    [from the cache]
©2003-2011 Jasper Spaans