Re: [PATCH] markup_oops.pl: add options to improve cross-sompilation environments

Hui Zhu
2010-02-01T05:37:53Z

Michal Marek:
> On 26.1.2010 10:13, Hui Zhu wrote:
>> +# Get options
>> +Getopt::Long::GetOptions(
>> + 'cross-compile|c=s' => \$cross_compile,
>> + 'module|m=s' => \$modulefile,
>> + 'help|h' => \&usage,
>> +);
>
> You should check the return code of GetOptions() and abort on invalid
> options.
>
>
>> +my $vmlinux_name = $ARGV[$#ARGV];
>
> GetOptions() deletes the recognized options from @ARGV, so you can say
> $ARGV[0] as before (and maybe check if there aren't any superfluous
> arguments).
>
>
>> # if it's a module, we need to find the .ko file and calculate a load
>> offset
>> if ($module ne "") {
>> - my $modulefile = `modinfo $module | grep '^filename:' | awk '{
>> print \$2 }'`;
>> - chomp($modulefile);
>> + if ($modulefile eq "") {
>> + my $modulefile = `modinfo $module | grep '^filename:' | awk '{
>> print \$2 }'`;
>
> I know you didn't add this, but while at it, could you replace the
> pipeline with just `modinfo -F filename $module`?
>
>
>> +sub usage {
>> + print <<EOT;
>> +Usage:
>> + dmesg | perl $0 [OPTION] [VMLINUX]
>> +
>> +OPTION:
>> + -c, --cross-compile CROSS_COMPILE Specify the prefix used for
>> toolchain.
>> + -m, --module MODULE_DIRNAME Specify the module directory name.
>
> Here and in the changelog you talk about "module directory name", but in
> fact this is the module filename.
>

Thanks Michael.

I make a patch according to your mail.

Best regards,
Hui

1. Fix a little format issue.
2. Check the return of "Getopt::Long::GetOptions". Output usage and exit if it get error.
3. Change $ARGV[$#ARGV] to $ARGV[0].
4. Change the code which get $modulefile from modinfo. Replace the pipeline with `modinfo -F filename $module`.
4. Change usage from "Specify the module directory name" to "Specify the module filename".

Signed-off-by: Hui Zhu <teawater@gmail.com>
---
scripts/markup_oops.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

--- a/scripts/markup_oops.pl
+++ b/scripts/markup_oops.pl
@@ -23,10 +23,10 @@ my $modulefile = "";
# Get options
Getopt::Long::GetOptions(
'cross-compile|c=s' => \$cross_compile,
- 'module|m=s' => \$modulefile,
+ 'module|m=s' => \$modulefile,
'help|h' => \&usage,
-);
-my $vmlinux_name = $ARGV[$#ARGV];
+) || usage ();
+my $vmlinux_name = $ARGV[0];
if (!defined($vmlinux_name)) {
my $kerver = `uname -r`;
chomp($kerver);
@@ -193,7 +193,7 @@ if ($target eq "0") {
# if it's a module, we need to find the .ko file and calculate a load offset
if ($module ne "") {
if ($modulefile eq "") {
- my $modulefile = `modinfo $module | grep '^filename:' | awk '{ print \$2 }'`;
+ $modulefile = `modinfo -F filename $module`;
chomp($modulefile);
}
$filename = $modulefile;
@@ -361,7 +361,7 @@ Usage:

OPTION:
-c, --cross-compile CROSS_COMPILE Specify the prefix used for toolchain.
- -m, --module MODULE_DIRNAME Specify the module directory name.
+ -m, --module MODULE_DIRNAME Specify the module filename.
-h, --help Help.
EOT
exit;