All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: devel@driverdev.osuosl.org
Cc: linux-kernel@vger.kernel.org
Subject: rename_rev.pl: review script for whitespace changes
Date: Fri, 26 Jun 2015 16:15:24 +0300	[thread overview]
Message-ID: <20150626131524.GQ30834@mwanda> (raw)

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

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

[-- Attachment #2: rename_rev.pl --]
[-- Type: text/x-perl, Size: 5392 bytes --]

#!/usr/bin/perl

# This is a tool to help review variable rename patches. The goal is
# to strip out the automatic sed renames and the white space changes
# and leaves the interesting code changes.
#
# Example 1: A patch renames openInfo to open_info:
#     cat diff | rename_review.pl openInfo open_info
#
# Example 2: A patch swaps the first two arguments to some_func():
#     cat diff | rename_review.pl \
#                    -e 's/some_func\((.*?),(.*?),/some_func\($2, $1,/'
#
# Example 3: A patch removes the xkcd_ prefix from some but not all the
# variables.  Instead of trying to figure out which variables were renamed
# just remove the prefix from them all:
#     cat diff | rename_review.pl -ea 's/xkcd_//g'
#
# Example 4: A patch renames 20 CamelCase variables.  To review this let's
# just ignore all case changes and all '_' chars.
#     cat diff | rename_review -ea 'tr/[A-Z]/[a-z]/' -ea 's/_//g'
#
# The other arguments are:
# -nc removes comments
# -ns removes '\' chars if they are at the end of the line.

use strict;
use File::Temp qw/ :mktemp  /;

sub usage() {
    print "usage: cat diff | $0 old new old new old new...\n";
    print "   or: cat diff | $0 -e 's/old/new/g'\n";
    print " -e : execute on old lines\n";
    print " -ea: execute on all lines\n";
    print " -nc: no comments\n";
    print " -nb: no unneeded braces\n";
    print " -ns: no slashes at the end of a line\n";
    print " -pull: for function pull.  deletes context.\n";
    exit(1);
}
my @subs;
my @cmds;
my $strip_comments;
my $strip_braces;
my $strip_slashes;
my $pull_context;

sub filter($) {
    my $_ = shift();
    my $old = 0;
    if ($_ =~ /^-/) {
        $old = 1;
    }
    # remove the first char
    s/^[ +-]//;
    if ($strip_comments) {
        s/\/\*.*?\*\///g;
        s/\/\/.*//;
    }
    foreach my $cmd (@cmds) {
        if ($old || $cmd->[0] =~ /^-ea$/) {
            eval $cmd->[1];
        }
    }
    foreach my $sub (@subs) {
        if ($old) {
            s/$sub->[0]/$sub->[1]/g;
        }
    }

    # remove the newline so we can move curly braces here if we want.
    s/\n//;
    return $_;
}

while (my $param1 = shift()) {
    if ($param1 =~ /^-nc$/) {
        $strip_comments = 1;
        next;
    }
    if ($param1 =~ /^-nb$/) {
        $strip_braces = 1;
        next;
    }
    if ($param1 =~ /^-ns$/) {
        $strip_slashes = 1;
        next;
    }
    if ($param1 =~ /^-pull$/) {
        $pull_context = 1;
        next;
    }
    my $param2 = shift();
    if ($param2 =~ /^$/) {
        usage();
    }
    if ($param1 =~ /^-e(a|)$/) {
        push @cmds, [$param1, $param2];
        next;
    }
    push @subs, [$param1, $param2];
}

my ($oldfh, $oldfile) = mkstemp("/tmp/oldXXXXX");
my ($newfh, $newfile) = mkstemp("/tmp/newXXXXX");

my $inside = 0;
my $output;

#recreate an old file and a new file
while (<>) {
    if ($pull_context && !($_ =~ /^[+-@]/)) {
        next;
    }

    if ($_ =~ /^(---|\+\+\+)/) {
        next;
    }

    if ($_ =~ /^@/) {
        $inside = 1;
    }
    if ($inside && !(($_ =~ /^[- @+]/) || ($_ =~ /^$/))) {
        $inside = 0;
    }
    if (!$inside) {
        next;
    }

    $output = filter($_);

    if ($strip_braces && $_ =~ /^(\+|-)\W+{/) {
        $output =~ s/^[\t ]+(.*)/ $1/;
    } else {
        $output = "\n" . $output;
    }

    if ($_ =~ /^-/) {
        print $oldfh $output;
        next;
    }
    if ($_ =~ /^\+/) {
        print $newfh $output;
        next;
    }
    print $oldfh $output;
    print $newfh $output;

}
print $oldfh "\n";
print $newfh "\n";
# git diff puts a -- and version at the end of the diff.  put the -- into the
# new file as well so it's ignored
if ($output =~ /\n-/) {
    print $newfh "-\n";
}

my $hunk;
my $old_txt;
my $new_txt;

open diff, "diff -uw $oldfile $newfile |";
while (<diff>) {
    if ($_ =~ /^(---|\+\+\+)/) {
        next;
    }

    if ($_ =~ /^@/) {

        if ($strip_comments) {
            $old_txt =~ s/\/\*.*?\*\///g;
            $new_txt =~ s/\/\*.*?\*\///g;
        }
        if ($strip_braces) {
            $old_txt =~ s/{([^;{]*?);}/$1;/g;
            $new_txt =~ s/{([^;{]*?);}/$1;/g;
            # this is a hack because i don't know how to replace nested
            # unneeded curly braces.
            $old_txt =~ s/{([^;{]*?);}/$1;/g;
            $new_txt =~ s/{([^;{]*?);}/$1;/g;
        }

        if ($old_txt ne $new_txt) {
            print $hunk;
            print $_;
        }
        $hunk = "";
        $old_txt = "";
        $new_txt = "";
        next;
    }

    $hunk = $hunk . $_;

    if ($strip_slashes) {
        s/\\$//;
    }

    if ($_ =~ /^-/) {
        s/-//;
        s/[ \t\n]//g;
        $old_txt = $old_txt . $_;
        next;
    }
    if ($_ =~ /^\+/) {
        s/\+//;
        s/[ \t\n]//g;
        $new_txt = $new_txt . $_;
        next;
    }
    if ($_ =~ /^ /) {
        s/^ //;
        s/[ \t\n]//g;
        $old_txt = $old_txt . $_;
        $new_txt = $new_txt . $_;
    }
}

if ($old_txt ne $new_txt) {
    if ($strip_comments) {
        $old_txt =~ s/\/\*.*?\*\///g;
        $new_txt =~ s/\/\*.*?\*\///g;
    }
    if ($strip_braces) {
        $old_txt =~ s/{([^;{]*?);}/$1;/g;
        $new_txt =~ s/{([^;{]*?);}/$1;/g;
        $old_txt =~ s/{([^;{]*?);}/$1;/g;
        $new_txt =~ s/{([^;{]*?);}/$1;/g;
    }

    print $hunk;
}

unlink($oldfile);
unlink($newfile);

print "\ndone.\n";

[-- Attachment #3: move_rev.pl --]
[-- Type: text/x-perl, Size: 3402 bytes --]

#!/usr/bin/perl

use strict;
use File::Temp qw/ tempdir /;
use File::Path qw/ rmtree /;
use File::Compare;

sub filter($) {
    my $_ = shift();

    # remove the first char
    s/^[ +-]//;
    return $_;
}

sub break_out_blocks($)
{
    my $dir = shift();
    my $block_nr = 0;

    open FILE, "<", "$dir/full";
    open OUT, ">", "$dir/$block_nr";

    while (<FILE>) {

        if ($block_nr && $_ =~ /^}/) {
            print OUT $_;
            close(OUT);
            $block_nr++;
            open OUT, ">", "$dir/$block_nr";
            next;
        }

        if ($_ =~ /^{/ || ($_ =~ /^[a-zA-Z]/ && $_ =~ / {$/)) {
            close(OUT);
            $block_nr++;
            open OUT, ">", "$dir/$block_nr";
        }

        print OUT $_;
    }
    close(OUT);
}

sub files_same($$)
{
    my $one = shift();
    my $two = shift();
    my $size_one = (stat($one))[7];
    my $size_two = (stat($two))[7];

    if ($size_one != $size_two) {
        return 0;
    }
    return compare($one, $two) == 0;
}

sub is_block($)
{
    my $file = shift();
    open LINE, "<", "$file";
    my $line = <LINE>;

    if ($line =~ /^{/ || ($line =~ /^[a-zA-Z]/ && $line =~ / {$/)) {
        return 1;
    }
    return 0;
}

sub replace_exact_blocks($$) {
    my $olddir = shift();
    my $newdir = shift();

    my $i = -1;
    while (1) {
        $i++;
        if (! -e "$olddir/$i") {
            last;
        }

        if (!is_block("$olddir/$i")) {
            next;
        }

        my $j = -1;
        while (1) {
            $j++;
            if (! -e "$newdir/$j") {
                last;
            }

            if (files_same("$olddir/$i", "$newdir/$j")) {

                open OUT, ">", "$olddir/$i";
                print OUT "- MOVED {$i}\n";
                close OUT;

                open OUT, ">", "$newdir/$j";
                print OUT "+ MOVED {$j}\n";
                close OUT;
                last;
            }

        }
    }
}

sub merge_blocks($) {
    my $dir = shift();

    open MERGED, ">", "$dir/merged";
    my $i = -1;
    while (1) {
        $i++;
        if (! -e "$dir/$i") {
            last;
        }

        open FILE, "<", "$dir/$i";
        while (<FILE>) {
            print MERGED $_;
        }
        close(FILE);
    }
    close(MERGED);
}

sub show_diff($$) {
    my $olddir = shift();
    my $newdir = shift();

    open diff, "diff -uw $olddir/merged $newdir/merged |";
    while (<diff>) {
        print $_;
    }
}

my $output;
my $inside = 0;
my $olddir = tempdir();
my $newdir = tempdir();

open oldfh, ">", "$olddir/full";
open newfh, ">", "$newdir/full";

#recreate an old file and a new file
while (<>) {
    if ($_ =~ /^(---|\+\+\+)/) {
        next;
    }

    if ($_ =~ /^@/) {
        $inside = 1;
    }
    if ($inside && !(($_ =~ /^[- @+]/) || ($_ =~ /^$/))) {
        $inside = 0;
    }
    if (!$inside) {
        next;
    }

    $output = filter($_);

    if ($_ =~ /^-/) {
        print oldfh $output;
        next;
    }
    if ($_ =~ /^\+/) {
        print newfh $output;
        next;
    }
    print oldfh $output;
    print newfh $output;

}

close(oldfh);
close(newfh);

break_out_blocks($olddir);
break_out_blocks($newdir);

replace_exact_blocks($olddir, $newdir);

merge_blocks($olddir);
merge_blocks($newdir);

show_diff($olddir, $newdir);

#print "old = $olddir/ new = $newdir/\n";
rmtree($olddir);
rmtree($newdir);

             reply	other threads:[~2015-06-26 13:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-26 13:15 Dan Carpenter [this message]
2015-06-26 17:07 ` rename_rev.pl: review script for whitespace changes Greg KH
2015-06-27  8:05 ` Sudip Mukherjee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150626131524.GQ30834@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.