From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752542AbbFZRHk (ORCPT ); Fri, 26 Jun 2015 13:07:40 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:45423 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751828AbbFZRHf (ORCPT ); Fri, 26 Jun 2015 13:07:35 -0400 X-Sasl-enc: kPrVzScMNsxjOjjjLe3iPJSG4nyZpT7a+Y82AF1vI5fi 1435338454 Date: Fri, 26 Jun 2015 10:07:32 -0700 From: Greg KH To: Dan Carpenter Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: rename_rev.pl: review script for whitespace changes Message-ID: <20150626170732.GA15099@kroah.com> References: <20150626131524.GQ30834@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150626131524.GQ30834@mwanda> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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