All of lore.kernel.org
 help / color / mirror / Atom feed
* Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
@ 2010-03-09 23:32 Joe Perches
  2010-03-09 23:41 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Joe Perches @ 2010-03-09 23:32 UTC (permalink / raw)
  To: LKML; +Cc: Greg Kroah-Hartman, devel, Andy Whitcroft

There was an article published recently:
http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
that seems to have prompted several new contributors (welcome)
to create style reformatting patches.

The article recommends running checkpatch and fixing the various
non-conforming style elements the output produces.

A better solution might be to enhance checkpatch to rewrite the
patch or a file with -f, but that's more than I'd like to do.

Maybe Andy Whitcroft might find a little of his copious spare
time available to start something like that.

For now, here's a little script that reformats source code to
be more a very little more kernel style compatible.

Maybe something like this could be useful to initial contributors
and could be extended as appropriate.

scripts/cvt_kernel_style.pl: Change a source file to more match kernel style

Trivial source file formatter.

Changes a few things to be more kernel style

Convert printk(KERN_<level> to pr_<level>(
Removes unnecessary parenthesis from return
Add space after if, for and while
Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
Removes multiple semicolons
Convert leading spaces to tabs

Use --source-indent to specify the #space->tab conversion (default 8)

Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/scripts/cvt_kernel_style.pl b/scripts/cvt_kernel_style.pl
new file mode 100755
index 0000000..39a8ab7
--- /dev/null
+++ b/scripts/cvt_kernel_style.pl
@@ -0,0 +1,134 @@
+#!/usr/bin/perl -w
+
+# Change some style elements of a source file
+# An imperfect source code rewriter.
+# Might make trivial patches a bit easier.
+#
+# usage: perl scripts/cvt_kernel_style.pl <files>
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+use strict;
+use Getopt::Long qw(:config no_auto_abbrev);
+
+my $P = $0;
+my $V = '0.1';
+
+my $source_indent = 8;
+
+my $version = 0;
+my $help = 0;
+
+if (!GetOptions(
+		'source-indent=i' => \$source_indent,
+		'v|version' => \$version,
+		'h|help|usage' => \$help,
+		)) {
+    die "$P: invalid argument - use --help if necessary\n";
+}
+
+if ($help != 0) {
+    usage();
+    exit 0;
+}
+
+sub usage {
+    print <<EOT;
+usage: $P [options] <files>
+version: $V
+
+Input source file descriptions:
+  --source-indent => How many spaces are used for an indent (default: 8)
+
+Other options:
+  --version => show version
+  --help => show this help information
+EOT
+}
+
+sub check_label {
+    my ($leading, $label) = @_;
+
+    if ($label == "default") {
+	return "$leading$label:";
+    }
+    return "$label:";
+}
+
+sub check_for {
+    my ($leading, $test1, $test2, $test3) = @_;
+
+    $test1 =~ s/^\s+|\s+$//g;
+    $test2 =~ s/^\s+|\s+$//g;
+    $test3 =~ s/^\s+|\s+$//g;
+
+    return "\n${leading}for ($test1; $test2; $test3)";
+}
+
+sub tabify {
+    my ($leading) = @_;
+
+    my $max_spaces_before_tab = $source_indent - 1;
+    my $spaces_to_tab = sprintf("%*s", $source_indent, "");
+#convert leading spaces to tabs
+    1 while $leading =~ s@^([\t]*)$spaces_to_tab@$1\t@g;
+#Remove spaces before a tab
+    1 while $leading =~ s@^([\t]*)([ ]{1,$max_spaces_before_tab})\t@$1\t@g;
+
+    return "\n$leading";
+}
+
+foreach my $file (@ARGV) {
+    my $f;
+    my $text;
+    my $oldtext;
+
+# read the file
+
+    open($f, '<', $file)
+	or die "$P: Can't open $file for read\n";
+    $oldtext = do { local($/) ; <$f> };
+    close($f);
+
+    $text = $oldtext;
+
+# Convert printk(KERN_<level> to pr_<level>(
+    $text =~ s@\bprintk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1\(@g;
+
+# Coalesce long formats
+    1 while $text =~ s@\b((dev_|pr_|netif_|netdev_)[^;]+)\"\s*\n\s*\"@$1@g;
+
+# Remove spaces and tabs before quoted newlines
+    $text =~ s@(\"[^\"]*)[ \t]+\\n@$1\\n@g;
+
+# Add space between KERN_<LEVEL> and open quote
+    $text =~ s@KERN_([A-Z]+)\"@KERN_$1 \"@g;
+
+# Remove unnecessary parentheses around return
+    $text =~ s@\breturn\s+\(([^\)]+)\s*\)\s*;@return $1;@g;
+
+## This doesn't work very well, too many comments modified
+## Put labels (but not "default:" on column 1
+#    $text =~ s@^([ \t]+)([A-Za-z0-9_]+)\s*:[ \t]*:[ \t]*$@check_label($1, $2)@ge;
+
+# Add space after (if, for, or while) and open parenthesis
+    $text =~ s@\b(if|while|for)\(@$1 \(@g;
+
+# Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
+    $text =~ s@\n([ \t]+)for\s*\([ \t]*([^;]+);[ \t]*([^;]+);[ \t]*([^\)]+)\)@check_for($1, $2, $3, $4)@ge;
+
+# Remove multiple semicolons at end-of-line
+    $text =~ s@;[ \t]*;[ \t]*\n@;\n@g;
+
+# Convert leading spaces to tabs
+    $text =~ s@\n([ \t]+)@tabify($1)@ge;
+
+# write the file if something was changed
+
+    if ($text ne $oldtext) {
+	open($f, '>', $file)
+	    or die "$P: Can't open $file for write\n";
+	print $f $text;
+	close($f);
+    }
+}




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-09 23:32 Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Joe Perches
@ 2010-03-09 23:41 ` Greg KH
  2010-03-10  0:06   ` Joe Perches
  2010-03-10  1:22 ` Frans Pop
  2010-03-10 18:32 ` Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Stefan Richter
  2 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2010-03-09 23:41 UTC (permalink / raw)
  To: Joe Perches; +Cc: LKML, devel, Andy Whitcroft

On Tue, Mar 09, 2010 at 03:32:06PM -0800, Joe Perches wrote:
> There was an article published recently:
> http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> that seems to have prompted several new contributors (welcome)
> to create style reformatting patches.
> 
> The article recommends running checkpatch and fixing the various
> non-conforming style elements the output produces.
> 
> A better solution might be to enhance checkpatch to rewrite the
> patch or a file with -f, but that's more than I'd like to do.

I _really_ dislike automatic source conversions by a tool and do not
recommend doing that at all.

It's better that you look at the code yourself, and make the change that
looks correct, which is not always the same thing that an automated tool
would do.

Becides, we have the Lindent script, which does almost exactly what you
are trying to do here, why reinvent the wheel?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-09 23:41 ` Greg KH
@ 2010-03-10  0:06   ` Joe Perches
  2010-03-10  0:16     ` Greg KH
  2010-03-10 18:35     ` Stefan Richter
  0 siblings, 2 replies; 12+ messages in thread
From: Joe Perches @ 2010-03-10  0:06 UTC (permalink / raw)
  To: Greg KH; +Cc: LKML, devel, Andy Whitcroft

On Tue, 2010-03-09 at 15:41 -0800, Greg KH wrote:
> On Tue, Mar 09, 2010 at 03:32:06PM -0800, Joe Perches wrote:
> > There was an article published recently:
> > http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> > that seems to have prompted several new contributors (welcome)
> > to create style reformatting patches.
> > 
> > The article recommends running checkpatch and fixing the various
> > non-conforming style elements the output produces.
> > 
> > A better solution might be to enhance checkpatch to rewrite the
> > patch or a file with -f, but that's more than I'd like to do.
> 
> I _really_ dislike automatic source conversions by a tool and do not
> recommend doing that at all.

Don't be silly.

You take automatic conversions from tools like coccinelle all
the time.

What I found poorly written about the article was
unfortunately you apparently recommend things like:

        if (error != -ENODEV) {
                foo();
                bar();
        } else {
        	report_error();
        	goto exit;
        }
        
instead of:

        if (error == -ENODEV) {
        	report_error();
        	goto exit;
        }
        foo();
        bar();

and the brace removal example was unfortunate because
it used printk without KERN_<level> without explanation.

I think there wasn't enough emphasis on compiling the new
patched file with something like a .o or .lst comparison
to the unmodified source to make sure it was OK to send
to the list.

> It's better that you look at the code yourself, and make the change that
> looks correct, which is not always the same thing that an automated tool
> would do.

I disagree.

It's better to make changes that _actually are_ correct.
People also neglect to see and convert a lot of things.

Tools can make coverage a bit better which is why a tool
like checkpatch exists in the first place.

> Becides, we have the Lindent script, which does almost exactly what you
> are trying to do here, why reinvent the wheel?

Lindent does a relatively poor job at conversions,
and it's all or nothing.  A tool that could be used
to selectively perform various conversions would be
better and could produce patches that are human
verifiable.

For instance, look at what happened with the old
reiserfs changes.

Lindent badly mucked up comments, did a poor job at
columnarization of variable declarations, didn't
understand c99 very well, was wretched at 80 columns,
etc.

I think the Lindent script should be deprecated and
checkpatch enhanced myself.  And until then, I think
a script like cvt_kernel_style.pl has some use.

cheers, Joe


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-10  0:06   ` Joe Perches
@ 2010-03-10  0:16     ` Greg KH
  2010-03-10  0:34       ` Joe Perches
  2010-03-10 18:35     ` Stefan Richter
  1 sibling, 1 reply; 12+ messages in thread
From: Greg KH @ 2010-03-10  0:16 UTC (permalink / raw)
  To: Joe Perches; +Cc: LKML, devel, Andy Whitcroft

On Tue, Mar 09, 2010 at 04:06:59PM -0800, Joe Perches wrote:
> On Tue, 2010-03-09 at 15:41 -0800, Greg KH wrote:
> > On Tue, Mar 09, 2010 at 03:32:06PM -0800, Joe Perches wrote:
> > > There was an article published recently:
> > > http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> > > that seems to have prompted several new contributors (welcome)
> > > to create style reformatting patches.
> > > 
> > > The article recommends running checkpatch and fixing the various
> > > non-conforming style elements the output produces.
> > > 
> > > A better solution might be to enhance checkpatch to rewrite the
> > > patch or a file with -f, but that's more than I'd like to do.
> > 
> > I _really_ dislike automatic source conversions by a tool and do not
> > recommend doing that at all.
> 
> Don't be silly.
> 
> You take automatic conversions from tools like coccinelle all
> the time.

Those are sane, as they are tiny and obvious :)

It's the huge "reformat the whole file with indent" type stuff that I
object to.

> What I found poorly written about the article was
> unfortunately you apparently recommend things like:
> 
>         if (error != -ENODEV) {
>                 foo();
>                 bar();
>         } else {
>         	report_error();
>         	goto exit;
>         }
>         
> instead of:
> 
>         if (error == -ENODEV) {
>         	report_error();
>         	goto exit;
>         }
>         foo();
>         bar();

Hey, a job as a copyeditor awaits :)

> and the brace removal example was unfortunate because
> it used printk without KERN_<level> without explanation.
> 
> I think there wasn't enough emphasis on compiling the new
> patched file with something like a .o or .lst comparison
> to the unmodified source to make sure it was OK to send
> to the list.

One can only do so much in a few thousand words.  I'm sorry you disliked
the article, I know the magazine is always looking for new authors.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-10  0:16     ` Greg KH
@ 2010-03-10  0:34       ` Joe Perches
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2010-03-10  0:34 UTC (permalink / raw)
  To: Greg KH; +Cc: LKML, devel, Andy Whitcroft

On Tue, 2010-03-09 at 15:41 -0800, Greg KH wrote:
> I _really_ dislike automatic source conversions by a tool and do not
> recommend doing that at all.
 
On Tue, 2010-03-09 at 16:16 -0800, Greg KH wrote:
> > You take automatic conversions from tools like coccinelle all
> > the time.
> Those are sane, as they are tiny and obvious :)

You contradict yourself.

> It's the huge "reformat the whole file with indent" type stuff
> that I object to.

Which is sensible and not what you lead with.

Any conversion done an automatic tool should be limited to
a specific type of change and should not be done wholesale.

Scripts like what I posted are easy to limit to specific
types of conversion.  Tools like Lindent are not.

cheers, Joe


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-09 23:32 Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Joe Perches
  2010-03-09 23:41 ` Greg KH
@ 2010-03-10  1:22 ` Frans Pop
  2010-03-10  3:25   ` [PATCH V2] scripts/cvt_kernel_style.pl: partial "kernel style" pretty-printing Joe Perches
  2010-03-10 18:32 ` Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Stefan Richter
  2 siblings, 1 reply; 12+ messages in thread
From: Frans Pop @ 2010-03-10  1:22 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, gregkh, devel, apw

Joe Perches wrote:
> The article recommends running checkpatch and fixing the various
> non-conforming style elements the output produces.

Hmm. I thought that "style cleanup only" patches were generally frowned 
upon? For one because it requires some familiarity with the kernel coding 
style to make sane choices in situations that are debatable and blindly 
following checkpatch is seldom good. And also to avoid needless merge 
issues.
I've seen several patches drift by the last few days where I thought some 
of the changes were definitely not improvements.

> Convert printk(KERN_<level> to pr_<level>(
> Removes unnecessary parenthesis from return
> Add space after if, for and while
> Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
> Removes multiple semicolons
> Convert leading spaces to tabs

Maybe I missed it, but you should certainly add removal of trailing space.
And possibly remove spaces before the closing ";" after statements.

Maybe the script should print a large warning (unless -q is used?) that all 
changes should be carefully reviewed manually and not combined with 
functional changes, and have a pointer to Documentation/SubmittingPatches?

Cheers,
FJP

P.S.
I wonder what traffic the advice to mail lkml when "I have a line of code 
that's over 80 chars" is going to generate...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2] scripts/cvt_kernel_style.pl: partial "kernel style" pretty-printing
  2010-03-10  1:22 ` Frans Pop
@ 2010-03-10  3:25   ` Joe Perches
  2010-03-24 20:01     ` [PATCH] scripts/cvt_kernel_style.pl: kernel style source code reformatter Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2010-03-10  3:25 UTC (permalink / raw)
  To: Frans Pop; +Cc: linux-kernel, gregkh, devel, apw

On Wed, 2010-03-10 at 02:22 +0100, Frans Pop wrote:
> Joe Perches wrote:
> > The article recommends running checkpatch and fixing the various
> > non-conforming style elements the output produces.
> Hmm. I thought that "style cleanup only" patches were generally frowned 
> upon?

Maybe so.
This article seemed geared to drivers/staging but it wasn't obvious.

> Maybe I missed it, but you should certainly add removal of trailing space.
> And possibly remove spaces before the closing ";" after statements.

OK, I can add that.

> Maybe the script should print a large warning (unless -q is used?) that all 
> changes should be carefully reviewed manually and not combined with 
> functional changes, and have a pointer to Documentation/SubmittingPatches?

> I wonder what traffic the advice to mail lkml when "I have a line of code 
> that's over 80 chars" is going to generate...

I think "get a life" was one, "come on" another.

Here's a new version that does individual style changes including
the trim and ; you suggested.
-----------------

Trivial source file reformatter.

Changes a few things to be more kernel style

Optionally convert any or all of:

Convert printk(KERN_<level> to pr_<level>(
Coalesces long format strings
Removes unnecessary parentheses from return
Add space after if, for and while
Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
Removes multiple semicolons
Convert leading spaces to tabs
Trims trailing whitespace
Moves labels to column 1

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/cvt_kernel_style.pl |  254 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 254 insertions(+), 0 deletions(-)

diff --git a/scripts/cvt_kernel_style.pl b/scripts/cvt_kernel_style.pl
new file mode 100755
index 0000000..fba6a9e
--- /dev/null
+++ b/scripts/cvt_kernel_style.pl
@@ -0,0 +1,254 @@
+#!/usr/bin/perl -w
+
+# Change some style elements of a source file.
+# An imperfect source code reformatter.
+# Might make trivial patches a bit easier.
+#
+# usage: perl scripts/cvt_kernel_style.pl <files>
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+use strict;
+use Getopt::Long qw(:config no_auto_abbrev);
+
+my $P = $0;
+my $V = '0.1';
+
+my $source_indent = 8;
+my $quiet = 0;
+my $overwrite = 0;
+my $modified = 0;
+my $suffix = ".style";
+my $convert_options = "";
+my @avail_options = ("all",
+		     "convert_printk_to_pr_level",
+		     "coalesce_formats",
+		     "convert_leading_spaces_to_tabs",
+		     "remove_trailing_whitespace",
+		     "deparenthesize_returns",
+		     "remove_unnecessary_semicolons",
+		     "remove_space_before_quoted_newline",
+		     "space_after_KERN_level",
+		     "space_after_if_while_for",
+		     "space_after_for_semicolons",
+		     "move_labels_to_column_1",
+		     "remove_whitespace_before_trailing_semicolon",
+		 );
+
+my $version = 0;
+my $help = 0;
+
+if (!GetOptions(
+		'source-indent=i' => \$source_indent,
+		'convert=s' => \$convert_options,
+		'o|overwrite!' => \$overwrite,
+		'q|quiet!' => \$quiet,
+		'v|version' => \$version,
+		'h|help|usage' => \$help,
+		)) {
+    die "$P: invalid argument - use --help if necessary\n";
+}
+
+if ($help) {
+    usage();
+    exit 0;
+}
+
+if ($version) {
+    print "$P: v$V\n";
+    exit 0;
+}
+
+sub usage {
+    print <<EOT;
+usage: $P [options] <files>
+version: $V
+
+EOT
+    print "Available conversions:\n";
+    foreach my $convert (@avail_options) {
+	print "\t$convert\n";
+    }
+    print "\n";
+    print "Use --convert=(comma separated list)\n";
+    print "   ie: --convert=convert_printk_to_pr_level,coalesce_formats\n";
+    print <<EOT;
+
+Input source file descriptions:
+  --source-indent => How many spaces are used for an indent (default: 8)
+
+Output file:
+  --overwrite => write the changes to the source file
+  --suffix => suffix to append to new file (default: .style)
+
+Other options:
+  --version => show version
+  --help => show this help information
+EOT
+}
+
+sub check_label {
+    my ($leading, $label) = @_;
+
+    if ($label == "default") {
+	return "$leading$label:";
+    }
+    return "$label:";
+}
+
+sub check_for {
+    my ($leading, $test1, $test2, $test3) = @_;
+
+    $test1 =~ s/^\s+|\s+$//g;
+    $test2 =~ s/^\s+|\s+$//g;
+    $test3 =~ s/^\s+|\s+$//g;
+
+    return "\n${leading}for ($test1; $test2; $test3)";
+}
+
+sub tabify {
+    my ($leading) = @_;
+
+    my $max_spaces_before_tab = $source_indent - 1;
+    my $spaces_to_tab = sprintf("%*s", $source_indent, "");
+#convert leading spaces to tabs
+    1 while $leading =~ s@^([\t]*)$spaces_to_tab@$1\t@g;
+#Remove spaces before a tab
+    1 while $leading =~ s@^([\t]*)([ ]{1,$max_spaces_before_tab})\t@$1\t@g;
+
+    return "\n$leading";
+}
+
+my %hash;
+foreach my $opt (@avail_options) {
+    $hash{$opt} = 0;
+}
+
+my @actual_options = split(',', $convert_options);
+foreach my $opt (@actual_options) {
+    $hash{$opt} = 1;
+}
+
+sub test {
+    my ($check) = @_;
+
+    return 1 if ($convert_options eq "all");
+    return 1 if ($hash{$check});
+
+    return 0;
+}
+
+foreach my $file (@ARGV) {
+    my $f;
+    my $text;
+    my $oldtext;
+
+# read the file
+
+    open($f, '<', $file)
+	or die "$P: Can't open $file for read\n";
+    $oldtext = do { local($/) ; <$f> };
+    close($f);
+
+    $text = $oldtext;
+
+# Convert printk(KERN_<level> to pr_<level>(
+    if (test("convert_printk_to_pr_level")) {
+	$text =~ s@\bprintk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1\(@g;
+    }
+
+# Coalesce long formats
+    if (test("coalesce_formats")) {
+	1 while $text =~ s@\b((dev_|pr_|netif_|netdev_)[^;]+)\"\s*\n\s*\"@$1@g;
+    }
+
+# Remove spaces and tabs before quoted newlines
+    if (test("remove space_before_quoted_newline")) {
+	$text =~ s@(\"[^\"]*)[ \t]+\\n@$1\\n@g;
+    }
+
+# Add space between KERN_<LEVEL> and open quote
+    if (test("space_after_KERN_level")) {
+	$text =~ s@\bKERN_([A-Z]+)[ \t]*\"@KERN_$1 \"@g;
+    }
+
+# Remove unnecessary parentheses around return
+    if (test("deparenthesize_returns")) {
+	$text =~ s@\breturn\s+\(([^\)]+)\s*\)\s*;@return $1;@g;
+    }
+
+# This doesn't work very well, too many comments modified
+# Put labels (but not "default:" on column 1
+    if (test("move_labels_to_column_1")) {
+	$text =~ s@^([ \t]+)([A-Za-z0-9_]+)\s*:[ \t]*:[ \t]*$@check_label($1, $2)@ge;
+    }
+
+# Add space after (if, for, or while) and open parenthesis
+    if (test("space_after_if_while_for")) {
+	$text =~ s@\b(if|while|for)\(@$1 \(@g;
+    }
+
+# Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
+    if (test("space_after_for_semicolons")) {
+	$text =~ s@\n([ \t]+)for\s*\([ \t]*([^;]+);[ \t]*([^;]+);[ \t]*([^\)]+)\)@check_for($1, $2, $3, $4)@ge;
+    }
+
+# Convert leading spaces to tabs
+    if (test("convert_leading_spaces_to_tabs")) {
+	$text =~ s@\n([ \t]+)@tabify($1)@ge;
+    }
+
+# Delete trailing whitespace
+    if (test("remove_trailing_whitespace")) {
+	$text =~ s@[ \t]+\n@\n@g;
+    }
+
+# Remove multiple semicolons at end-of-line
+    if (test("remove_multiple_semicolons")) {
+	$text =~ s@;[ \t]*;[ \t]*\n@;\n@g;
+    }
+
+# Delete whitespace before trailing semicolon
+    if (test("remove_whitespace_before_trailing_semicolon")) {
+	$text =~ s@[ \t]*;[ \t]*\n@;\n@g;
+    }
+
+# write the file if something was changed
+
+    if ($text ne $oldtext) {
+	my $newfile = $file;
+	if (!$overwrite) {
+	    $newfile = "$newfile$suffix";
+	}
+	open($f, '>', $newfile)
+	    or die "$P: Can't open $newfile for write\n";
+	print $f $text;
+	close($f);
+
+	if (!$quiet) {
+	    if ($overwrite) {
+		print "Converted $file\n";
+	    } else {
+		print "Converted $file to $newfile\n";
+	    }
+	    $modified = 1;
+	}
+    }
+}
+
+if ($modified && !$quiet) {
+    print <<EOT;
+
+Warning: these changes may not be correct.
+
+These changes should be carefully reviewed manually and not combined with
+any functional changes.
+
+Compile, build and test your changes.
+
+You should understand and be responsible for all object changes.
+
+Make sure you read Documentation/SubmittingPatches before sending
+any changes to reviewers, maintainers or mailing lists.
+EOT
+}



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-09 23:32 Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Joe Perches
  2010-03-09 23:41 ` Greg KH
  2010-03-10  1:22 ` Frans Pop
@ 2010-03-10 18:32 ` Stefan Richter
  2 siblings, 0 replies; 12+ messages in thread
From: Stefan Richter @ 2010-03-10 18:32 UTC (permalink / raw)
  To: Joe Perches; +Cc: LKML, Greg Kroah-Hartman, devel, Andy Whitcroft

Joe Perches wrote:
> There was an article published recently:
> http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
> that seems to have prompted several new contributors (welcome)
> to create style reformatting patches.

Uh oh.  How can a newcomer know what good style is?

Please start your career of kernel contributions differently:
  - Test new kernels.  Report issues and work with the developers to
    get the issues fixed.
  - If there is a bug which nags you but there doesn't seem to be
    an active developer to fix it anytime soon, try to fix it yourself.

Or if you are aware of a small RFE for a subsystem which interests you
personally, try to implement that small feature.

It doesn't make sense to start kernel development with something which
you are likely not going to connect with personally.  (Reformat others'
code?  Better write your own code, i.e. a bug fix or a small feature.)

PS:
I am also saying this because I repeatedly see "style fixes" posted
which are questionable at best, or actually degrade style, or even
introduce bugs.

PPS:
How can a script know what good style is?
-- 
Stefan Richter
-=====-==-=- --== -=-=-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-10  0:06   ` Joe Perches
  2010-03-10  0:16     ` Greg KH
@ 2010-03-10 18:35     ` Stefan Richter
  2010-03-10 18:38       ` Joe Perches
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Richter @ 2010-03-10 18:35 UTC (permalink / raw)
  To: Joe Perches; +Cc: Greg KH, LKML, devel, Andy Whitcroft

Joe Perches wrote:
> On Tue, 2010-03-09 at 15:41 -0800, Greg KH wrote:
>> It's better that you look at the code yourself, and make the change that
>> looks correct, which is not always the same thing that an automated tool
>> would do.
> 
> I disagree.
> 
> It's better to make changes that _actually are_ correct.
> People also neglect to see and convert a lot of things.

There is no correct and incorrect style.
There is only
  - accepted/ not accepted style,
  - good/ bad style.
-- 
Stefan Richter
-=====-==-=- --== -=-=-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl
  2010-03-10 18:35     ` Stefan Richter
@ 2010-03-10 18:38       ` Joe Perches
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2010-03-10 18:38 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Greg KH, LKML, devel, Andy Whitcroft

On Wed, 2010-03-10 at 19:35 +0100, Stefan Richter wrote:
> Joe Perches wrote:
> > It's better to make changes that _actually are_ correct.
> There is no correct and incorrect style.
> There is only
>   - accepted/ not accepted style,
>   - good/ bad style.

I meant to avoid style changes that introduce defects
or don't even compile.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] scripts/cvt_kernel_style.pl: kernel style source code reformatter
  2010-03-10  3:25   ` [PATCH V2] scripts/cvt_kernel_style.pl: partial "kernel style" pretty-printing Joe Perches
@ 2010-03-24 20:01     ` Joe Perches
  2010-03-24 22:31       ` Frans Pop
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2010-03-24 20:01 UTC (permalink / raw)
  To: linux-kernel; +Cc: Frans Pop, Greg KH, devel, apw, Andrew Morton

A script to convert kernel source files to a more conformant style.
A supplement to or replacement of Lindent.
A wretched little perl script using regexes.

It's a stupid little tool, don't expect it to be perfect.  It's not.

Conversions should be done one at a time.
Multiple conversions may be performed together, but it's not recommended.

Not all conversions are performed correctly.
Verify all conversions before committing anything.

If the original source file doesn't compile, then any conversion will not
compile either and may eat your source.

Do not use option --overwrite unless you have another copy of the source file.

No option exists to wrap long lines.

Command line use:

$ ./scripts/cvt_kernel_style.pl --help
usage: ./scripts/cvt_kernel_style.pl [options] <files>
version: 0.1

Available conversions:
	all
 	convert_printk_to_pr_level
	coalesce_formats
	cuddle_open_brace
	cuddle_else
	deparenthesize_returns
	space_after_KERN_level
	space_after_if_while_for_switch
	space_after_for_semicolons
	space_after_comma
	space_before_pointer
	space_around_trigraph
	convert_leading_spaces_to_tabs
	coalesce_semicolons
	remove_trailing_whitespace
	remove_whitespace_before_quoted_newline
	remove_whitespace_before_trailing_semicolon
	remove_whitespace_before_bracket
	remove_parenthesis_whitespace
	remove_single_statement_braces
	hoist_assigns_from_if
	convert_c99_comments
Additional conversions which may not work well:
	(enable individually or with --convert=all --broken)
	move_labels_to_column_1
	space_around_logical_tests
	space_around_assign
	space_around_arithmetic

Use --convert=(comma separated list)
   ie: --convert=convert_printk_to_pr_level,coalesce_formats

Input source file descriptions:
  --source-indent => How many spaces are used for an indent (default: 8)

Output file:
  --overwrite => write the changes to the source file
  --suffix => suffix to append to new file (default: .style)

Other options:
  --quiet => don't show conversion warning messages (default: disabled)
  --stats => show conversions done (default: enabled)
  --version => show version
  --help => show this help information

For instance:

$ ./scripts/cvt_kernel_style.pl --convert=hoist_assigns_from_if \
	-o --stats --quiet \
	drivers/net/tulip/de2104x.c
Converted drivers/net/tulip/de2104x.c
1	hoist_assigns_from_if
$ git diff drivers/net/tulip/de2104x.c
(added) diff --git a/drivers/net/tulip/de2104x.c b/drivers/net/tulip/de2104x.c
index cb42972..0cb9f38 100644
--- a/drivers/net/tulip/de2104x.c
+++ b/drivers/net/tulip/de2104x.c
@@ -2153,7 +2153,8 @@ static int de_resume (struct pci_dev *pdev)
                goto out;
        if (!netif_running(dev))
                goto out_attach;
-       if ((retval = pci_enable_device(pdev))) {
+       retval = pci_enable_device(pdev);
+       if (retval) {
                dev_err(&dev->dev, "pci_enable_device failed in resume\n");
                goto out;
        }

Conversions done badly or a seriously broken manner:

The script doesn't ignore comments, comments will be reformatted.
This may be undesired and should be fixed-up by hand.
C99 comment conversion can occur within comments.

Conversions can occur within quoted strings.
This may be undesired and should be fixed-up by hand.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/cvt_kernel_style.pl |  478 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 478 insertions(+), 0 deletions(-)
 create mode 100755 scripts/cvt_kernel_style.pl

diff --git a/scripts/cvt_kernel_style.pl b/scripts/cvt_kernel_style.pl
new file mode 100755
index 0000000..350c1b0
--- /dev/null
+++ b/scripts/cvt_kernel_style.pl
@@ -0,0 +1,478 @@
+#!/usr/bin/perl -w
+
+# Change some style elements of a source file
+# An imperfect source code formatter.
+# Might make trivial patches a bit easier.
+#
+# usage: perl scripts/cvt_kernel_style.pl <files>
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+use strict;
+use Getopt::Long qw(:config no_auto_abbrev);
+
+my $P = $0;
+my $V = '0.1';
+
+my $source_indent = 8;
+my $quiet = 0;
+my $stats = 1;
+my $overwrite = 0;
+my $modified = 0;
+my $suffix = ".style";
+my $convert_options = "";
+my $broken = 0;
+
+my @std_options = (
+    "all",
+    "convert_printk_to_pr_level",
+    "coalesce_formats",
+    "cuddle_open_brace",
+    "cuddle_else",
+    "deparenthesize_returns",
+    "space_after_KERN_level",
+    "space_after_if_while_for_switch",
+    "space_after_for_semicolons",
+    "space_after_comma",
+    "space_before_pointer",
+    "space_around_trigraph",
+    "convert_leading_spaces_to_tabs",
+    "coalesce_semicolons",
+    "remove_trailing_whitespace",
+    "remove_whitespace_before_quoted_newline",
+    "remove_whitespace_before_trailing_semicolon",
+    "remove_whitespace_before_bracket",
+    "remove_parenthesis_whitespace",
+    "remove_single_statement_braces",
+    "hoist_assigns_from_if",
+    "convert_c99_comments",
+);
+
+my @other_options = (
+    "move_labels_to_column_1",
+    "space_around_logical_tests",
+    "space_around_assign",
+    "space_around_arithmetic",
+);
+
+my $version = 0;
+my $help = 0;
+
+my $logFunctions = qr{(?x:
+	printk|
+	pr_(debug|dbg|vdbg|devel|info|warning|err|notice|alert|crit|emerg|cont)|
+	dev_(printk|dbg|vdbg|info|warn|err|notice|alert|crit|emerg|WARN)|
+	netdev_(printk|dbg|vdbg|info|warn|err|notice|alert|crit|emerg|WARN)|
+	netif_(printk|dbg|vdbg|info|warn|err|notice|alert|crit|emerg|WARN)|
+	WARN|
+	panic
+)};
+
+my $match_balanced_parentheses = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+my $do_cvt;
+
+my %hash;
+
+sub set_all_options {
+    my ($enabled) = @_;
+
+    foreach my $opt (@std_options) {
+	$hash{$opt} = $enabled;
+    }
+
+    if ($broken > 0 || $enabled == -1) {
+	foreach my $opt (@other_options) {
+	    $hash{$opt} = $enabled;
+	}
+    }
+
+}
+
+if (!GetOptions(
+		'source-indent=i' => \$source_indent,
+		'convert=s' => \$convert_options,
+		'broken!' => \$broken,
+		'stats!' => \$stats,
+		'o|overwrite!' => \$overwrite,
+		'q|quiet!' => \$quiet,
+		'v|version' => \$version,
+		'h|help|usage' => \$help,
+		)) {
+    die "$P: invalid argument - use --help if necessary\n";
+}
+
+if ($help) {
+    usage();
+    exit 0;
+}
+
+if ($version) {
+    print "$P: v$V\n";
+    exit 0;
+}
+
+my $max_spaces_before_tab = $source_indent - 1;
+my $spaces_to_tab = sprintf("%*s", $source_indent, "");
+
+set_all_options(-1);
+
+my @actual_options = split(',', $convert_options);
+foreach my $opt (@actual_options) {
+    if ($opt eq "all") {
+	set_all_options(0);
+    }
+    if (exists($hash{$opt})) {
+	$hash{$opt} = 0;
+    } else {
+	print "Invalid --convert option: '$opt', ignored\n";
+    }
+}
+
+sub usage {
+    print <<EOT;
+usage: $P [options] <files>
+version: $V
+
+EOT
+    print "Available conversions:\n";
+    foreach my $convert (@std_options) {
+	print "\t$convert\n";
+    }
+    print "Additional conversions which may not work well:\n";
+    print "\t(enable individually or with --convert=all --broken)\n";
+    foreach my $convert (@other_options) {
+	print "\t$convert\n";
+    }
+    print "\n";
+    print "Use --convert=(comma separated list)\n";
+    print "   ie: --convert=convert_printk_to_pr_level,coalesce_formats\n";
+    print <<EOT;
+
+Input source file descriptions:
+  --source-indent => How many spaces are used for an indent (default: 8)
+
+Output file:
+  --overwrite => write the changes to the source file
+  --suffix => suffix to append to new file (default: .style)
+
+Other options:
+  --quiet => don't show conversion warning messages (default: disabled)
+  --stats => show conversions done (default: enabled)
+  --version => show version
+  --help => show this help information
+EOT
+}
+
+sub check_label {
+    my ($leading, $label) = @_;
+
+    if ($label == "default") {
+	return "$leading$label:";
+    }
+    return "$label:";
+}
+
+sub check_for {
+    my ($leading, $test1, $test2, $test3) = @_;
+
+    $test1 =~ s/^\s+|\s+$//g;
+    $test2 =~ s/^\s+|\s+$//g;
+    $test3 =~ s/^\s+|\s+$//g;
+
+    return "${leading}for ($test1; $test2; $test3)";
+}
+
+sub tabify {
+    my ($leading) = @_;
+
+#convert leading spaces to tabs
+    1 while $leading =~ s@^([\t]*)$spaces_to_tab@$1\t@g;
+#Remove spaces before a tab
+    1 while $leading =~ s@^([\t]*)([ ]{1,$max_spaces_before_tab})\t@$1\t@g;
+
+    return "$leading";
+}
+
+sub default_substitute {
+    my ($argument) = @_;
+
+    return "$argument";
+}
+
+sub subst_line_mode_fn {
+    my ($lines, $match, $fn, $args) = @_;
+
+    my $function = \&$fn;
+    my @lines = split("\n", $lines);
+    my $count = 0;
+
+    foreach my $line (@lines) {
+	my $oldline = $line;
+	$line =~ s@$match@&$function(eval $args)@ge;
+	$count++ if ($oldline ne $line);
+    }
+
+    return ($count, join("\n", @lines) . "\n");
+}
+
+sub subst_line_mode {
+    my ($lines, $match, $substitute) = @_;
+
+    return subst_line_mode_fn($lines, $match, "default_substitute", $substitute);
+}
+
+sub convert {
+    my ($check) = @_;
+
+    return 1 if ($hash{$check} >= 0);
+
+    return 0;
+}
+
+foreach my $file (@ARGV) {
+    my $f;
+    my $text;
+    my $oldtext;
+
+# read the file
+
+    open($f, '<', $file)
+	or die "$P: Can't open $file for read\n";
+    $oldtext = do { local($/) ; <$f> };
+    close($f);
+
+    $text = $oldtext;
+
+# Convert printk(KERN_<level> to pr_<level>(
+    $do_cvt = "convert_printk_to_pr_level";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bprintk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1\(@g;
+    }
+
+# Coalesce long formats
+    $do_cvt = "coalesce_formats";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@\b(${logFunctions}\s*\([^;]+)\"\s*\n\s*\"@$1@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Add space between KERN_<LEVEL> and open quote
+    $do_cvt = "space_after_KERN_level";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\b(KERN_(DEBUG|INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)) \"@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\b(KERN_(DEBUG|INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT))[ \t]*\"@$1 \"@g;
+    }
+
+# Remove unnecessary parentheses around return
+    $do_cvt = "deparenthesize_returns";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@\breturn\s+\(([^\)]+)\s*\)\s*;@return $1;@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# This doesn't work very well, too many comments modified
+# Put labels (but not "default:") on column 1
+    $do_cvt = "move_labels_to_column_1";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@^([ \t]+)([A-Za-z0-9_]+)\s*:[ \t]*:[ \t]*$@check_label($1, $2)@ge;
+    }
+
+# Add space after (if, while, for, switch) and open parenthesis
+    $do_cvt = "space_after_if_while_for_switch";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\b(if|while|for|switch) \(@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\b(if|while|for|switch)[ \t]*\(@$1 \(@g;
+    }
+
+# Add space after comma
+    $do_cvt = "space_after_comma";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@,(?=[\w\(])@, @g;
+    }
+
+# Add spaces around logical tests
+    $do_cvt = "space_around_logical_tests";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@([\)\w]+)(==|!=|>|>=|<|<=)([\(\w\*\-])@$1 $2 $3@g;
+    }
+
+# Add spaces around assign
+    $do_cvt = "space_around_assign";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@([\)\w]+)(=|\+=|\-=|\*=|/=|>>=|<<=)([\(\w\*\-])@$1 $2 $3@g;
+    }
+
+# Add spaces around arithmetic
+    $do_cvt = "space_around_arithmetic";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@([\)\w]+)(\+|\-)([\(\w\*])@$1 $2 $3@g;
+    }
+
+# Add spaces around trigraph
+    $do_cvt = "space_around_trigraph";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@([\)\w\"]+) \? ([\(\)\[\]\w\*\" \t\.\>\-]*[^ \t]) \: ([\w\(\"\-])@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@([\)\w\"]+)[ \t]*\?[ \t]*([\(\)\[\]\w\*\" \t\.\>\-]*[^ \t])[ \t]*\:[ \t]*([\w\(\"\-])@$1 ? $2 : $3@g;
+    }
+
+# Use a space before a pointer,
+    $do_cvt = "space_before_pointer";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\bstruct \w+ \*@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+)([\t]+)\*[ \t]*@struct $1$2\*@g;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+) *\*[ \t]*@struct $1 \*@g;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+)([ \t]+)\*__@struct $1$2\* __@g;
+    }
+
+# Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
+    $do_cvt = "space_after_for_semicolons";
+    if (convert($do_cvt)) {
+	my $count;
+	($count, $text) = subst_line_mode_fn($text, '^([ \t]*)for\s*\([ \t]*([^;]+);[ \t]*([^;]+);[ \t]*([^\)]+)\)', 'check_for', '$1, $2, $3, $4');
+	$hash{$do_cvt} += $count;
+    }
+
+# cuddle open brace
+    $do_cvt = "cuddle_open_brace";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@(\)|\belse\b) \{\n@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@(\)|\belse\b)[ \t]*[ \t]*\n[ \t]+\{[ \t]*\n@$1 \{\n@g;
+    }
+
+# cuddle else
+    $do_cvt = "cuddle_else";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\} else\b@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\}[ \t]*\n[ \t]+else\b@\} else@g;
+    }
+
+# Remove multiple semicolons at end-of-line
+    $do_cvt = "coalesce_semicolons";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@;[ \t]*;[ \t]*\n@;\n@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Remove spaces before open bracket
+    $do_cvt = "remove_whitespace_before_bracket";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]+\[@\[@g;
+    }
+
+# Remove spaces after open parenthesis and before close parenthesis
+    $do_cvt = "remove_parenthesis_whitespace";
+    if (convert($do_cvt)) {
+	$text =~ s@[ \t]*\)@\)@g;
+	$text =~ s@\([ \t]*@\(@g;
+    }
+
+# Convert leading spaces to tabs
+    $do_cvt = "convert_leading_spaces_to_tabs";
+    if (convert($do_cvt)) {
+	my $count;
+	($count, $text) = subst_line_mode_fn($text, '(^[ \t]+)', 'tabify', '$1');
+	$hash{$do_cvt} += $count;
+    }
+
+# Remove trailing whitespace
+    $do_cvt = "remove_trailing_whitespace";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]+\n@\n@g;
+    }
+
+# Remove whitespace before quoted newlines
+    $do_cvt = "remove_whitespace_before_quoted_newline";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(\"[^\"\n]*[^ \t])[ \t]+\\n@$1\\n@g;
+    }
+
+# Remove whitespace before trailing semicolon
+    $do_cvt = "remove_whitespace_before_trailing_semicolon";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(\n[^\n]+)\s+;[ \t]*\n$@$1;\n@g;
+    }
+
+# Convert c99 comments to /* */ (don't convert (http|ftp)://)
+    $do_cvt = "convert_c99_comments";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(?<!:)\/\/[ \t]*(.*)[ \t]*\n+@\/* $1 *\/\n@g;
+    }
+
+# Remove braces from single statements (not multiple-line single statements)
+    $do_cvt = "remove_single_statement_braces";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]*\{[ \t]*\n([^;\{\n]+;)[ \t]*\n[ \t]+\}[ \t]*\n@\n$1\n@g;
+    }
+
+# Hoist assigns from if
+    $do_cvt = "hoist_assigns_from_if";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\n([ \t]*)if\s*\(\s*([\!]{0,1})\s*\(\s*([\*\w\-\>\.\[\]]+)\s*=\s*(?=[^=])\s*([\w\-\>\.\* \t\[\]]*\s*${match_balanced_parentheses}*\s*(\?\:\&|\||\>\>|\<\<|\-|\+|\*|\/ \t)*\s*[\w\-\>\.\* \t\[\]]*\s*${match_balanced_parentheses}*)\s*\)@\n$1$3 = $4;\n$1if \($2$3@gx;
+    }
+
+# write the file if something was changed
+
+    if ($text ne $oldtext) {
+	my $newfile = $file;
+
+	$modified = 1;
+
+	if (!$overwrite) {
+	    $newfile = "$newfile$suffix";
+	}
+	open($f, '>', $newfile)
+	    or die "$P: Can't open $newfile for write\n";
+	print $f $text;
+	close($f);
+
+	if (!$quiet || $stats) {
+	    if ($overwrite) {
+		print "Converted $file\n";
+	    } else {
+		print "Converted $file to $newfile\n";
+	    }
+	}
+
+	if ($stats) {
+	    while ((my $key, my $value) = each(%hash)) {
+		next if ($value <= 0);
+		print "$value\t$key\n" if $value;
+		$hash{$key} = 0;	#Reset for next file
+	    }
+	}
+
+    }
+}
+
+
+if ($modified && !$quiet) {
+    print <<EOT;
+
+Warning: these changes may not be correct.
+
+These changes should be carefully reviewed manually and not combined with
+any functional changes.
+
+Compile, build and test your changes.
+
+You should understand and be responsible for all object changes.
+
+Make sure you read Documentation/SubmittingPatches before sending
+any changes to reviewers, maintainers or mailing lists.
+EOT
+}
-- 
1.7.0.14.g7e948


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] scripts/cvt_kernel_style.pl: kernel style source code reformatter
  2010-03-24 20:01     ` [PATCH] scripts/cvt_kernel_style.pl: kernel style source code reformatter Joe Perches
@ 2010-03-24 22:31       ` Frans Pop
  0 siblings, 0 replies; 12+ messages in thread
From: Frans Pop @ 2010-03-24 22:31 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, Greg KH, devel, apw, Andrew Morton

On Wednesday 24 March 2010, Joe Perches wrote:
> A script to convert kernel source files to a more conformant style.
> A supplement to or replacement of Lindent.
> A wretched little perl script using regexes.

I've found the script useful, but it does require some experience and 
judgement to use correctly.

For example, with my 'trailing spaces in messages' patches I've found it
is essential to review all changes to avoid e.g. changing output to files 
in /proc. Doing that can be valid, but it probably should not be included 
in the same patch as removing trailing space from kernel messages.

Given that experience and judgement is needed to use many tools correctly,

Supported-by: Frans Pop <elendil@planet.nl>

Cheers,
FJP

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-03-24 22:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-09 23:32 Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Joe Perches
2010-03-09 23:41 ` Greg KH
2010-03-10  0:06   ` Joe Perches
2010-03-10  0:16     ` Greg KH
2010-03-10  0:34       ` Joe Perches
2010-03-10 18:35     ` Stefan Richter
2010-03-10 18:38       ` Joe Perches
2010-03-10  1:22 ` Frans Pop
2010-03-10  3:25   ` [PATCH V2] scripts/cvt_kernel_style.pl: partial "kernel style" pretty-printing Joe Perches
2010-03-24 20:01     ` [PATCH] scripts/cvt_kernel_style.pl: kernel style source code reformatter Joe Perches
2010-03-24 22:31       ` Frans Pop
2010-03-10 18:32 ` Tuxradar patching article and [PATCH] scripts/cvt_kernel_style.pl Stefan Richter

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.