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    [W:0.044 / U:90.324 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site