linux-kernel - Re: update checkpatch.pl to version 0.19

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080609083301.GA6701@shadowen.org>
Date:	Mon, 9 Jun 2008 09:33:01 +0100
From:	Andy Whitcroft <apw@...dowen.org>
To:	Greg KH <greg@...ah.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-usb@...r.kernel.org
Subject: Re: 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@...ah.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/