[PATCH 0/3] fixes, improvements on scripts/export_report.pl

[PATCH 0/3] fixes, improvements on scripts/export_report.pl

[Jim Cromie]

[PATCH 1/3] do collectcfiles work in perl itself, eschew shell pipeline
[PATCH 2/3] sort SECTION 2 output of scripts/export_report.pl
[PATCH 3/3] in scripts/export_report.pl use warn() to issue WARNING, so they go to stderr.

also available at git://github.com/jimc/linux-2.6.git export-report branch

[PATCH 1/3] do collectcfiles work in perl itself, eschew shell pipeline

[Jim Cromie]

avoid spawning a shell pipeline doing cat, grep, sed,
and do it all inside perl.  The <*.c> construct works
at least as far back as 5.8.9

Note that this is not just an optimization; the sed command
in the pipeline was unterminated, due to lack of escape on the
end-of-line (\$) in the regex, resulting in this:

    $ perl ../linux-2.6/scripts/export_report.pl  > /dev/null
    sed: -e expression #1, char 5: unterminated `s' command
    sh: .mod.c/: not found

Comments on an earlier patch sought an all-perl implementation.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
cc: Michal Marek <mmarek@suse.cz>,
cc: linux-kbuild@vger.kernel.org
cc: Arnaud Lacombe lacombar@gmail.com
cc: Stephen Hemminger shemminger@vyatta.com
---
 scripts/export_report.pl |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index 04dce7c..5499ff0 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -49,8 +49,14 @@ sub usage {
 }
 
 sub collectcfiles {
-    my @file
-	= `cat .tmp_versions/*.mod | grep '.*\.ko\$' | sed s/\.ko$/.mod.c/`;
+    my @file;
+    while (<.tmp_versions/*.mod>) {
+	open my $fh, '<', $_ or die "cant open $_: $!\n";
+	push (@file,
+	      grep s/\.ko/.mod.c/,	# change the suffix
+	      grep m/.+\.ko/,		# find the .ko path
+	      <$fh>);			# lines in opened file
+    }
     chomp @file;
     return @file;
 }
-- 
1.7.4.1

[PATCH 2/3] sort SECTION 2 output of scripts/export_report.pl

[Jim Cromie]

Sort SECTION 2 modules by name.  Within those module listings, sort
the symbol providers by name, and remove the count, as it is
misleading; its the kernel-wide count of uses of that symbol, not the
count pertaining to the module being outlined.  (this can be seen by
grepping the output for a single symbol).  The count is still used to
sort the symbols.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/export_report.pl |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index 5499ff0..16bd240 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -25,11 +25,12 @@ sub alphabetically {
 sub print_depends_on {
 	my ($href) = @_;
 	print "\n";
-	while (my ($mod, $list) = each %$href) {
+	for my $mod (sort keys %$href) {
+		my $list = $href->{$mod};
 		print "\t$mod:\n";
 		foreach my $sym (sort numerically @{$list}) {
 			my ($symbol, $no) = split /\s+/, $sym;
-			printf("\t\t%-25s\t%-25d\n", $symbol, $no);
+			printf("\t\t%-25s\n", $symbol);
 		}
 		print "\n";
 	}
@@ -166,7 +167,8 @@ modules. Each module lists the modules, and the symbols from that module that
 it uses.  Each listed symbol reports the number of modules using it\n");
 
 print "~"x80 , "\n";
-while (my ($thismod, $list) = each %MODULE) {
+for my $thismod (sort keys %MODULE) {
+	my $list = $MODULE{$thismod};
 	my %depends;
 	$thismod =~ s/\.mod\.c/.ko/;
 	print "\t\t\t$thismod\n";
-- 
1.7.4.1

[PATCH 3/3] in scripts/export_report.pl use warn() to issue WARNING, so they go to stderr.

[Jim Cromie]

Also count CONFIG_MODVERSION warnings, and print an NB message at
start of SECTION 2 if any were issued.  Section 2 will be empty
if the build is lacking this CONFIG_ item, and user may have missed
the warnings, as they're off screen.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/export_report.pl |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index 16bd240..14c1638 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -102,6 +102,8 @@ close($module_symvers);
 #
 # collect the usage count of each symbol.
 #
+my $modversion_warnings = 0;
+
 foreach my $thismod (@allcfiles) {
 	my $module;
 
@@ -132,7 +134,8 @@ foreach my $thismod (@allcfiles) {
 		}
 	}
 	if ($state != 2) {
-		print "WARNING:$thismod is not built with CONFIG_MODVERSION enabled\n";
+		warn "WARNING:$thismod is not built with CONFIG_MODVERSION enabled\n";
+		$modversion_warnings++;
 	}
 	close($module);
 }
@@ -166,6 +169,9 @@ printf("SECTION 2:\n\tThis section reports export-symbol-usage of in-kernel
 modules. Each module lists the modules, and the symbols from that module that
 it uses.  Each listed symbol reports the number of modules using it\n");
 
+print "\nNB: Got $modversion_warnings CONFIG_MODVERSION warnings\n\n"
+    if $modversion_warnings;
+
 print "~"x80 , "\n";
 for my $thismod (sort keys %MODULE) {
 	my $list = $MODULE{$thismod};
-- 
1.7.4.1

Re: [PATCH 1/3] do collectcfiles work in perl itself, eschew shell pipeline

[Randy Dunlap]

On Sat, May 21, 2011 1:21 am, Jim Cromie wrote:

>  sub collectcfiles {
> -    my @file
> -	= `cat .tmp_versions/*.mod | grep '.*\.ko\$' | sed s/\.ko$/.mod.c/`;
> +    my @file;
> +    while (<.tmp_versions/*.mod>) {
> +	open my $fh, '<', $_ or die "cant open $_: $!\n";

Use either "can't" or "cannot".  "cant" is a different word.

> +	push (@file,
> +	      grep s/\.ko/.mod.c/,	# change the suffix
> +	      grep m/.+\.ko/,		# find the .ko path
> +	      <$fh>);			# lines in opened file
> +    }
>      chomp @file;
>      return @file;
>  }


-- 
~Randy

Re: [PATCH 1/3] do collectcfiles work in perl itself, eschew shell pipeline

[Randy Dunlap]

On Sat, May 21, 2011 1:21 am, Jim Cromie wrote:

>  sub collectcfiles {
> -    my @file
> -	= `cat .tmp_versions/*.mod | grep '.*\.ko\$' | sed s/\.ko$/.mod.c/`;
> +    my @file;
> +    while (<.tmp_versions/*.mod>) {
> +	open my $fh, '<', $_ or die "cant open $_: $!\n";

Use either "can't" or "cannot".  "cant" is a different word.

> +	push (@file,
> +	      grep s/\.ko/.mod.c/,	# change the suffix
> +	      grep m/.+\.ko/,		# find the .ko path
> +	      <$fh>);			# lines in opened file
> +    }
>      chomp @file;
>      return @file;
>  }


-- 
~Randy

Re: [PATCH 3/3] in scripts/export_report.pl use warn() to issue WARNING, so they go to stderr.

[Randy Dunlap]

On Sat, 21 May 2011 02:21:48 -0600 Jim Cromie wrote:

> Also count CONFIG_MODVERSION warnings, and print an NB message at

CONFIG_MODVERSIONS

NB?  as in Note Well?

Please just use NOTE:

> start of SECTION 2 if any were issued.  Section 2 will be empty
> if the build is lacking this CONFIG_ item, and user may have missed
> the warnings, as they're off screen.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  scripts/export_report.pl |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/scripts/export_report.pl b/scripts/export_report.pl
> index 16bd240..14c1638 100644
> --- a/scripts/export_report.pl
> +++ b/scripts/export_report.pl
> @@ -102,6 +102,8 @@ close($module_symvers);
>  #
>  # collect the usage count of each symbol.
>  #
> +my $modversion_warnings = 0;
> +
>  foreach my $thismod (@allcfiles) {
>  	my $module;
>  
> @@ -132,7 +134,8 @@ foreach my $thismod (@allcfiles) {
>  		}
>  	}
>  	if ($state != 2) {
> -		print "WARNING:$thismod is not built with CONFIG_MODVERSION enabled\n";
> +		warn "WARNING:$thismod is not built with CONFIG_MODVERSION enabled\n";

should be CONFIG_MODVERSIONS

> +		$modversion_warnings++;
>  	}
>  	close($module);
>  }
> @@ -166,6 +169,9 @@ printf("SECTION 2:\n\tThis section reports export-symbol-usage of in-kernel
>  modules. Each module lists the modules, and the symbols from that module that
>  it uses.  Each listed symbol reports the number of modules using it\n");
>  
> +print "\nNB: Got $modversion_warnings CONFIG_MODVERSION warnings\n\n"

should be CONFIG_MODVERSIONS

and NOTE:


> +    if $modversion_warnings;
> +
>  print "~"x80 , "\n";
>  for my $thismod (sort keys %MODULE) {
>  	my $list = $MODULE{$thismod};
> -- 


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

[PATCH 0/3 rev2] fix scripts/export_report.pl

[Jim Cromie]

thanks Randy,

rev2 fixes:
NB -> NOTE
cant -> cannot
CONFIG_MODVERSION -> CONFIG_MODVERSIONS (everywhere)

[PATCH 1/3] do collectcfiles work in perl itself, eschew shell pipeline

[Jim Cromie]

Avoid spawning a shell pipeline doing cat, grep, sed, and do it all
inside perl.  The <*.c> globbing construct works at least as far back
as 5.8.9

Note that this is not just an optimization; the sed command
in the pipeline was unterminated, due to lack of escape on the
end-of-line (\$) in the regex, resulting in this:

    $ perl ../linux-2.6/scripts/export_report.pl  > /dev/null
    sed: -e expression #1, char 5: unterminated `s' command
    sh: .mod.c/: not found

Comments on an earlier patch sought an all-perl implementation.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
cc: Michal Marek <mmarek@suse.cz>,
cc: linux-kbuild@vger.kernel.org
cc: Arnaud Lacombe lacombar@gmail.com
cc: Stephen Hemminger shemminger@vyatta.com
---
 scripts/export_report.pl |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index 04dce7c..f97899c 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -49,8 +49,14 @@ sub usage {
 }
 
 sub collectcfiles {
-    my @file
-	= `cat .tmp_versions/*.mod | grep '.*\.ko\$' | sed s/\.ko$/.mod.c/`;
+    my @file;
+    while (<.tmp_versions/*.mod>) {
+	open my $fh, '<', $_ or die "cannot open $_: $!\n";
+	push (@file,
+	      grep s/\.ko/.mod.c/,	# change the suffix
+	      grep m/.+\.ko/,		# find the .ko path
+	      <$fh>);			# lines in opened file
+    }
     chomp @file;
     return @file;
 }
-- 
1.7.4.4

[PATCH 2/3] sort SECTION 2 output of scripts/export_report.pl

[Jim Cromie]

Sort SECTION 2 modules by name.  Within those module listings, sort
the symbol providers by name, and remove the count, as it is
misleading; its the kernel-wide count of uses of that symbol, not the
count pertaining to the module being outlined.  (this can be seen by
grepping the output for a single symbol).  The count is still used to
sort the symbols.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/export_report.pl |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index f97899c..48398a1 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -25,11 +25,12 @@ sub alphabetically {
 sub print_depends_on {
 	my ($href) = @_;
 	print "\n";
-	while (my ($mod, $list) = each %$href) {
+	for my $mod (sort keys %$href) {
+		my $list = $href->{$mod};
 		print "\t$mod:\n";
 		foreach my $sym (sort numerically @{$list}) {
 			my ($symbol, $no) = split /\s+/, $sym;
-			printf("\t\t%-25s\t%-25d\n", $symbol, $no);
+			printf("\t\t%-25s\n", $symbol);
 		}
 		print "\n";
 	}
@@ -166,7 +167,8 @@ modules. Each module lists the modules, and the symbols from that module that
 it uses.  Each listed symbol reports the number of modules using it\n");
 
 print "~"x80 , "\n";
-while (my ($thismod, $list) = each %MODULE) {
+for my $thismod (sort keys %MODULE) {
+	my $list = $MODULE{$thismod};
 	my %depends;
 	$thismod =~ s/\.mod\.c/.ko/;
 	print "\t\t\t$thismod\n";
-- 
1.7.4.4

[PATCH 3/3] in scripts/export_report.pl use warn() to issue WARNING, so they go to stderr.

[Jim Cromie]

Also count CONFIG_MODVERSIONS warnings, and print a NOTE at start of
SECTION 2 if any were issued.  Section 2 will be empty if the build is
lacking this CONFIG_ item, and user may have missed the warnings, as
they're off screen.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/export_report.pl |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/scripts/export_report.pl b/scripts/export_report.pl
index 48398a1..8f79b70 100644
--- a/scripts/export_report.pl
+++ b/scripts/export_report.pl
@@ -102,6 +102,8 @@ close($module_symvers);
 #
 # collect the usage count of each symbol.
 #
+my $modversion_warnings = 0;
+
 foreach my $thismod (@allcfiles) {
 	my $module;
 
@@ -132,7 +134,8 @@ foreach my $thismod (@allcfiles) {
 		}
 	}
 	if ($state != 2) {
-		print "WARNING:$thismod is not built with CONFIG_MODVERSION enabled\n";
+		warn "WARNING:$thismod is not built with CONFIG_MODVERSIONS enabled\n";
+		$modversion_warnings++;
 	}
 	close($module);
 }
@@ -166,6 +169,9 @@ printf("SECTION 2:\n\tThis section reports export-symbol-usage of in-kernel
 modules. Each module lists the modules, and the symbols from that module that
 it uses.  Each listed symbol reports the number of modules using it\n");
 
+print "\nNOTE: Got $modversion_warnings CONFIG_MODVERSIONS warnings\n\n"
+    if $modversion_warnings;
+
 print "~"x80 , "\n";
 for my $thismod (sort keys %MODULE) {
 	my $list = $MODULE{$thismod};
-- 
1.7.4.4

Re: [PATCH 0/3 rev2] fix scripts/export_report.pl

[Michal Marek]

On Mon, May 23, 2011 at 12:44:54PM -0600, Jim Cromie wrote:
> thanks Randy,
> 
> rev2 fixes:
> NB -> NOTE
> cant -> cannot
> CONFIG_MODVERSION -> CONFIG_MODVERSIONS (everywhere)

Applied to kbuild-2.6.git#misc after editing the subject lines a bit:

Jim Cromie (3):
      export_report: do collectcfiles work in perl itself
      export_report: sort SECTION 2 output
      export_report: use warn() to issue WARNING, so they go to stderr

Michal

Re: [PATCH 0/3 rev2] fix scripts/export_report.pl

[Jim Cromie]

On Tue, May 24, 2011 at 8:10 AM, Michal Marek <mmarek@suse.cz> wrote:

> Applied to kbuild-2.6.git#misc after editing the subject lines a bit:
>
> Jim Cromie (3):
>      export_report: do collectcfiles work in perl itself
>      export_report: sort SECTION 2 output
>      export_report: use warn() to issue WARNING, so they go to stderr
>
> Michal
>

thanks,

can you point to the tree ?
(I want to see what rewording is better)

I didnt see it at git.kernel.org, nor where MAINTAINERS entry points
(at least after converting it to http)

T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild-2.6.git
for-next
T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild-2.6.git
rc-fixes

Re: [PATCH 0/3 rev2] fix scripts/export_report.pl

[Michal Marek]

On 24.5.2011 19:53, Jim Cromie wrote:
> can you point to the tree ?
> (I want to see what rewording is better)
>
> I didnt see it at git.kernel.org, nor where MAINTAINERS entry points
> (at least after converting it to http)

http://git.kernel.org/?p=linux/kernel/git/mmarek/kbuild-2.6.git;a=shortlog;h=refs/heads/misc

Michal