smatch_scripts/whitespace_only.sh

smatch_scripts/whitespace_only.sh

[Dan Carpenter]

I wrote a script to check that a patch only changes white space.
It compiles the files before and after the patch is applied and 
verifies that they are the same.

You'll need to compile smatch:
	git pull git://repo.or.cz/smatch.git
	make
	cd /path/to/kernel/src/
	/path/to/smatch_scripts/whitespace_only.sh <patch>

Adding or removing parenthesis and curly braces counts as a code 
change.  Changes to comments, #if 0, white space changes do not.

You can fix a lot of style violations if you limit yourself to 
adding and removing tabs, spaces and new lines.  Then the next 
patch could remove unneeded parenthesis.  It would be easier to
audit that way instead of everything mixed together.

regards,
dan carpenter

PS.  I feel really bad slagging the newbies trying to help.  
Could we fix checkpatch.pl to not complain about line lengths?
Also could we tell them to stay in staging where no one cares
about git blame?

Re: smatch_scripts/whitespace_only.sh

[Julia Lawall]

On Wed, 10 Mar 2010, Dan Carpenter wrote:

> I wrote a script to check that a patch only changes white space.
> It compiles the files before and after the patch is applied and 
> verifies that they are the same.
> 
> You'll need to compile smatch:
> 	git pull git://repo.or.cz/smatch.git
> 	make
> 	cd /path/to/kernel/src/
> 	/path/to/smatch_scripts/whitespace_only.sh <patch>
> 
> Adding or removing parenthesis and curly braces counts as a code 
> change.  Changes to comments, #if 0, white space changes do not.
> 
> You can fix a lot of style violations if you limit yourself to 
> adding and removing tabs, spaces and new lines.  Then the next 
> patch could remove unneeded parenthesis.  It would be easier to
> audit that way instead of everything mixed together.
> 
> regards,
> dan carpenter
> 
> PS.  I feel really bad slagging the newbies trying to help.  
> Could we fix checkpatch.pl to not complain about line lengths?

I hope not...  Are newbies really put off by having to add a newline here 
and there?

julia

> Also could we tell them to stay in staging where no one cares
> about git blame?
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: smatch_scripts/whitespace_only.sh

[Dan Carpenter]

On Wed, Mar 10, 2010 at 11:14:53AM +0100, Julia Lawall wrote:
> On Wed, 10 Mar 2010, Dan Carpenter wrote:
> 
> > I wrote a script to check that a patch only changes white space.
> > It compiles the files before and after the patch is applied and 
> > verifies that they are the same.
> > 
> > You'll need to compile smatch:
> > 	git pull git://repo.or.cz/smatch.git
> > 	make
> > 	cd /path/to/kernel/src/
> > 	/path/to/smatch_scripts/whitespace_only.sh <patch>
> > 
> > Adding or removing parenthesis and curly braces counts as a code 
> > change.  Changes to comments, #if 0, white space changes do not.
> > 
> > You can fix a lot of style violations if you limit yourself to 
> > adding and removing tabs, spaces and new lines.  Then the next 
> > patch could remove unneeded parenthesis.  It would be easier to
> > audit that way instead of everything mixed together.
> > 
> > regards,
> > dan carpenter
> > 
> > PS.  I feel really bad slagging the newbies trying to help.  
> > Could we fix checkpatch.pl to not complain about line lengths?
> 
> I hope not...  Are newbies really put off by having to add a newline here 
> and there?
> 
> julia
> 

The problem is that newbies are only too happy to chop lines up into smaller 
and smaller bits.  :P  I've seen situations where the actual author of the code
broke up _his own_ perfectly reasonable code into tiny chunks just to satisfy 
checkpatch.pl.

				printk("blah blah blah blah"
						"blah blah"
						"blah");

It would be great if people dealt with checkpatch warnings by breaking
things out into separate functions and eliminating indent levels but 
that never happens.

regards,
dan carpenter

ps: Emacs starts with an 80 character window by default.  On my login
I have a ~/bin/emacs shell script that invoke real emacs as full screen.

#!/bin/sh
/usr/bin/emacs -fh -fw --no-splash $@

Re: smatch_scripts/whitespace_only.sh

[Julia Lawall]

On Wed, 10 Mar 2010, Dan Carpenter wrote:

> On Wed, Mar 10, 2010 at 11:14:53AM +0100, Julia Lawall wrote:
> > On Wed, 10 Mar 2010, Dan Carpenter wrote:
> > 
> > > I wrote a script to check that a patch only changes white space.
> > > It compiles the files before and after the patch is applied and 
> > > verifies that they are the same.
> > > 
> > > You'll need to compile smatch:
> > > 	git pull git://repo.or.cz/smatch.git
> > > 	make
> > > 	cd /path/to/kernel/src/
> > > 	/path/to/smatch_scripts/whitespace_only.sh <patch>
> > > 
> > > Adding or removing parenthesis and curly braces counts as a code 
> > > change.  Changes to comments, #if 0, white space changes do not.
> > > 
> > > You can fix a lot of style violations if you limit yourself to 
> > > adding and removing tabs, spaces and new lines.  Then the next 
> > > patch could remove unneeded parenthesis.  It would be easier to
> > > audit that way instead of everything mixed together.
> > > 
> > > regards,
> > > dan carpenter
> > > 
> > > PS.  I feel really bad slagging the newbies trying to help.  
> > > Could we fix checkpatch.pl to not complain about line lengths?
> > 
> > I hope not...  Are newbies really put off by having to add a newline here 
> > and there?
> > 
> > julia
> > 
> 
> The problem is that newbies are only too happy to chop lines up into smaller 
> and smaller bits.  :P  I've seen situations where the actual author of the code
> broke up _his own_ perfectly reasonable code into tiny chunks just to satisfy 
> checkpatch.pl.
> 
> 				printk("blah blah blah blah"
> 						"blah blah"
> 						"blah");
> 
> It would be great if people dealt with checkpatch warnings by breaking
> things out into separate functions and eliminating indent levels but 
> that never happens.

Maybe checkpatch could be changed to not complain about > 80 characters 
when the line contains a string?

julia

Re: smatch_scripts/whitespace_only.sh

[Joe Perches]

On Wed, 2010-03-10 at 15:38 +0100, Julia Lawall wrote:
> > > On Wed, 10 Mar 2010, Dan Carpenter wrote:
> > > > I wrote a script to check that a patch only changes white space.

Perhaps you could use #define __LINE__ 0 to avoid
any vertical line position comparison checks?

> Maybe checkpatch could be changed to not complain about > 80 characters 
> when the line contains a string?

That's been at least partially done in commit
691e669ba8c64d31ac08d87b1751e6acfa3ff65e

Re: smatch_scripts/whitespace_only.sh

[Dan Carpenter]

On Wed, Mar 10, 2010 at 10:11:49AM -0800, Joe Perches wrote:
> On Wed, 2010-03-10 at 15:38 +0100, Julia Lawall wrote:
> > > > On Wed, 10 Mar 2010, Dan Carpenter wrote:
> > > > > I wrote a script to check that a patch only changes white space.
> 
> Perhaps you could use #define __LINE__ 0 to avoid
> any vertical line position comparison checks?
> 

I tried something similar to that at first but I found that line 
numbers were used in other ways.  In the end, I added a -no-lineno to
sparse so it thinks everything is on line 123456.

It might be better to just right a perl script that strips out all the
whitespace and compares the result blobs before and after the patch.  
That would handle the #if 0 code as well.

regards,
dan carpenter

> > Maybe checkpatch could be changed to not complain about > 80 characters 
> > when the line contains a string?
> 
> That's been at least partially done in commit
> 691e669ba8c64d31ac08d87b1751e6acfa3ff65e