linux-kernel - Re: rename_rev.pl: review script for whitespace changes

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]

Date:	Fri, 26 Jun 2015 10:07:32 -0700
From:	Greg KH <greg@...ah.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: rename_rev.pl: review script for whitespace changes

On Fri, Jun 26, 2015 at 04:15:24PM +0300, Dan Carpenter wrote:
> I've sent my review script out a few times before but we have some new
> reviewers in staging who maybe haven't tried them.
> 
> rename_rev.pl strips out whitespace changes.  We recently had someone
> send a re-indent patch that deleted a line of code by mistake.  The diff
> looked like:
> 
> 18 files changed, 906 insertions(+), 927 deletions(-)
> 
> It would be difficult to spot the bug manually but when you cat it
> through rename_rev.pl then it stands out immediately.
> 
> If the patch changes // comments to /* */ then `rename_rev.pl -nc`
> strips out most of the comments.
> 
> If the patch re-indents macros then the -ns removes slashes from the
> ends of lines.
> 
> Sometimes people pull out some code into a separate function.  The -pull
> option is supposed to help for that.  It sometimes does...
> 
> The other thing that we see a lot in staging is when people change curly
> braces around.  The -nb option removes curly brace changes.
> 
> Another thing is we had a change which did this:
> 
> -#define HOST_IF_MSG_SCAN                        ((u16)0)
> -(40 lines of similar code)
> +#define HOST_IF_MSG_SCAN                        0
> +(40 lines of similar code)
> 
> I used rename_rev.pl -e 's/\(\(u16\)(.*)\)/$1/' to make sure nothing
> else changed.
> 
> Or if you are making hundreds of functions "static", then I just remove
> all the statics by doing rename_rev.pl -ea 's/static//'.  The -ea option
> stands for Execute on All.
> 
> Oh.  And I am also going to include my move_rev.pl, script.  That is for
> if we move functions from one file to another.
> 
> cat patch | move_rev.pl | rename_rev.pl
> 
> The rename_rev.pl script is sort of crappy, but often it removes a lot
> of stuff.  It doesn't have to be perfect to be better, I guess.
> 
> What I wish is that there were an -auto option which would find which
> variables were renamed.  Oh! Oh!  I have left out the most important
> feature.  Say you are renaming variables from SomeName to some_name then
> cat patch | rename_rev.pl SomeName some_name TwoName two_name Foo foo
> 
> regards,
> dan carpenter

Thanks a lot for these, much appreciated, I'll work to add them to my
test scripts for patches.

greg k-h
--
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/