From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754157Ab0BAFhx (ORCPT ); Mon, 1 Feb 2010 00:37:53 -0500 Received: from mail.windriver.com ([147.11.1.11]:40410 "EHLO mail.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750707Ab0BAFhv (ORCPT ); Mon, 1 Feb 2010 00:37:51 -0500 Message-ID: <4B666982.1020007@windriver.com> Date: Mon, 01 Feb 2010 13:41:22 +0800 From: Hui Zhu User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: Michal Marek CC: =?UTF-8?B?QW3DqXJpY28gV2FuZw==?= , Andrew Morton , Arjan van de Ven , Sam Ravnborg , ozan@pardus.org.tr, Matthew Wilcox , linux-kernel@vger.kernel.org, teawater@gmail.com Subject: Re: [PATCH] markup_oops.pl: add options to improve cross-sompilation environments References: <4B5E5D4F.50803@windriver.com> <2375c9f91001251905p6ed99405m3915ac56e230605f@mail.gmail.com> <4B5E9BE9.2030702@windriver.com> <2375c9f91001252353u52f437b0t7ddd643a57047e1c@mail.gmail.com> <4B5EB223.60307@windriver.com> <4B6344CA.10504@suse.cz> In-Reply-To: <4B6344CA.10504@suse.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 01 Feb 2010 05:29:57.0800 (UTC) FILETIME=[97A4CA80:01CAA2FF] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 <> +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 --- 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;