A little feature addition to allow checkpatch.pl to check patches piped
into it, in addition to specific file arguments.
Signed-off-by: Daniel Walker <[email protected]>
---
scripts/checkpatch.pl | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
Index: linux-2.6.23/scripts/checkpatch.pl
===================================================================
--- linux-2.6.23.orig/scripts/checkpatch.pl
+++ linux-2.6.23/scripts/checkpatch.pl
@@ -24,6 +24,7 @@ my $file = 0;
my $check = 0;
my $summary = 1;
my $mailback = 0;
+my $piped = (-t STDIN) ? 0 : 1;
my $root;
GetOptions(
'q|quiet+' => \$quiet,
@@ -43,7 +44,7 @@ GetOptions(
my $exit = 0;
-if ($#ARGV < 0) {
+if ($#ARGV < 0 && !$piped) {
print "usage: $P [options] patchfile\n";
print "version: $V\n";
print "options: -q => quiet\n";
@@ -181,6 +182,18 @@ if ($tree && -f "$root/$removal") {
}
my @rawlines = ();
+
+if ($piped) {
+ while (<STDIN>) {
+ chomp;
+ push(@rawlines, $_);
+ }
+ if (!process("", @rawlines)) {
+ $exit = 1;
+ }
+ @rawlines = ();
+}
+
for my $filename (@ARGV) {
if ($file) {
open(FILE, "diff -u /dev/null $filename|") ||
--
--
On 01/11/2008 05:10 AM, Daniel Walker wrote:
> A little feature addition to allow checkpatch.pl to check patches piped
> into it, in addition to specific file arguments.
You can still add - as an argument to check stdin. In which way is this better?
regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> > A little feature addition to allow checkpatch.pl to check patches piped
> > into it, in addition to specific file arguments.
>
> You can still add - as an argument to check stdin. In which way is this better?
There's was no reason to limit the arguments ..
I was using it to do something like the following ,
git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl
Which I think is useful .
Dainel
On 01/11/2008 10:17 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
>> On 01/11/2008 05:10 AM, Daniel Walker wrote:
>>> A little feature addition to allow checkpatch.pl to check patches piped
>>> into it, in addition to specific file arguments.
>> You can still add - as an argument to check stdin. In which way is this better?
>
> There's was no reason to limit the arguments ..
>
> I was using it to do something like the following ,
>
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl
Ok, and if you add a - there, it should have the same effect, but for free,
doesn't it:
git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl -
regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> >>> A little feature addition to allow checkpatch.pl to check patches piped
> >>> into it, in addition to specific file arguments.
> >> You can still add - as an argument to check stdin. In which way is this better?
> >
> > There's was no reason to limit the arguments ..
> >
> > I was using it to do something like the following ,
> >
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl
>
> Ok, and if you add a - there, it should have the same effect, but for free,
> doesn't it:
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl -
JftSoC:
git show 9914cad54c79d0b89b1f066c0894f00e1344131c | ./scripts/checkpatch.pl /dev/stdin
(and there a a few others) should also do the trick.
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> > On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> > >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
> > >>> A little feature addition to allow checkpatch.pl to check
> patches piped
> > >>> into it, in addition to specific file arguments.
> > >> You can still add - as an argument to check stdin. In which way
> is this better?
> > >
> > > There's was no reason to limit the arguments ..
> > >
> > > I was using it to do something like the following ,
> > >
> > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl
> >
> > Ok, and if you add a - there, it should have the same effect, but
> for free,
> > doesn't it:
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl -
>
> JftSoC:
> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> | ./scripts/checkpatch.pl /dev/stdin
> (and there a a few others) should also do the trick.
Not a particularly attractive command line .. Might still be a good idea
to add this since these two forms alluded me, and are likely to allude
people new to unix all together (who is more likely to be using this
particular tool)..
Daniel
On 01/11/2008 10:30 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
>> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> | ./scripts/checkpatch.pl -
>>
>> JftSoC:
>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> | ./scripts/checkpatch.pl /dev/stdin
>> (and there a a few others) should also do the trick.
>
> Not a particularly attractive command line .. Might still be a good idea
> to add this since these two forms alluded me, and are likely to allude
> people new to unix all together (who is more likely to be using this
> particular tool)..
If somebody is hacking kernel, I think he should know the - trick used in many
programs, but do not consider this as a nack.
thanks,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:30 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> >> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> >>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> >> | ./scripts/checkpatch.pl -
> >>
> >> JftSoC:
> >> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> >> | ./scripts/checkpatch.pl /dev/stdin
> >> (and there a a few others) should also do the trick.
> >
> > Not a particularly attractive command line .. Might still be a good idea
> > to add this since these two forms alluded me, and are likely to allude
> > people new to unix all together (who is more likely to be using this
> > particular tool)..
>
> If somebody is hacking kernel, I think he should know the - trick used in many
> programs, but do not consider this as a nack.
I'm hacking the kernel, and I didn't know the - trick .. So you have
your testing case all in one with the patch submitter ..
Daniel
On 01/11/2008 10:36 AM, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>> If somebody is hacking kernel, I think he should know the - trick used in many
>> programs, but do not consider this as a nack.
>
> I'm hacking the kernel, and I didn't know the - trick .. So you have
> your testing case all in one with the patch submitter ..
OK, maybe we should base it in doc for patch submitting then?
regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
On Fri, 2008-01-11 at 10:41 +0100, Jiri Slaby wrote:
> On 01/11/2008 10:36 AM, Daniel Walker wrote:
> > On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> >> If somebody is hacking kernel, I think he should know the - trick used in many
> >> programs, but do not consider this as a nack.
> >
> > I'm hacking the kernel, and I didn't know the - trick .. So you have
> > your testing case all in one with the patch submitter ..
>
> OK, maybe we should base it in doc for patch submitting then?
It's in the bash docs I'm sure, it's just a matter of who ends up
reading that part of the docs.. I still think my patch is a good one ,
we could change the name to "Allow piping with out tricks" I suppose ..
Daniel
On Fre, 2008-01-11 at 01:30 -0800, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:23 +0100, Bernd Petrovitsch wrote:
> > On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
> > > On 01/11/2008 10:17 AM, Daniel Walker wrote:
> > > > On Fri, 2008-01-11 at 09:52 +0100, Jiri Slaby wrote:
> > > >> On 01/11/2008 05:10 AM, Daniel Walker wrote:
[...]
> > > > I was using it to do something like the following ,
> > > >
> > > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl
> > >
> > > Ok, and if you add a - there, it should have the same effect, but
> > for free,
> > > doesn't it:
> > > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl -
> >
> > JftSoC:
> > git show 9914cad54c79d0b89b1f066c0894f00e1344131c
> > | ./scripts/checkpatch.pl /dev/stdin
> > (and there a a few others) should also do the trick.
>
> Not a particularly attractive command line .. Might still be a good idea
Might be.
But it works for (almost?) all programs (which do not need seekable
input) which absolutely want the input file specified by a parameter.
And if it's an proprietary program, you can't even patch it.
> to add this since these two forms alluded me, and are likely to allude
> people new to unix all together (who is more likely to be using this
> particular tool)..
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
On Fre, 2008-01-11 at 01:47 -0800, Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:41 +0100, Jiri Slaby wrote:
> > On 01/11/2008 10:36 AM, Daniel Walker wrote:
> > > On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
> > >> If somebody is hacking kernel, I think he should know the - trick used in many
> > >> programs, but do not consider this as a nack.
> > >
> > > I'm hacking the kernel, and I didn't know the - trick .. So you have
> > > your testing case all in one with the patch submitter ..
> >
> > OK, maybe we should base it in doc for patch submitting then?
>
> It's in the bash docs I'm sure, it's just a matter of who ends up
Yes, too. And "awk" also supports that.
But "/dev/stdin" is also since ages a symlink to
"/proc/self/fd/0" (which is another equivalent "file").
> reading that part of the docs.. I still think my patch is a good one ,
> we could change the name to "Allow piping with out tricks" I suppose ..
Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services
Daniel Walker wrote:
> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>> >> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>> >>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>> >> | ./scripts/checkpatch.pl -
>> If somebody is hacking kernel, I think he should know the - trick used in many
>> programs, but do not consider this as a nack.
>
> I'm hacking the kernel, and I didn't know the - trick .. So you have
> your testing case all in one with the patch submitter ..
How about
if ($#ARGV < 0) {
print "usage: $P [options] patchfile\n";
print "version: $V\n";
print "options: -q => quiet\n";
print " --no-tree => run without a kernel tree\n";
print " --terse => one line per report\n";
print " --emacs => emacs compile window format\n";
print " --file => check a source file\n";
print " --strict => enable more subjective tests\n";
print " --root => path to the kernel tree root\n";
+ print "When patchfile is -, read standard input.\n";
exit(1);
}
--
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/
On 01/11/2008 12:16 PM, Stefan Richter wrote:
> Daniel Walker wrote:
>> On Fri, 2008-01-11 at 10:34 +0100, Jiri Slaby wrote:
>>>>> On Fre, 2008-01-11 at 10:21 +0100, Jiri Slaby wrote:
>>>>>> git show 9914cad54c79d0b89b1f066c0894f00e1344131c
>>>>> | ./scripts/checkpatch.pl -
>
>>> If somebody is hacking kernel, I think he should know the - trick used in many
>>> programs, but do not consider this as a nack.
>> I'm hacking the kernel, and I didn't know the - trick .. So you have
>> your testing case all in one with the patch submitter ..
>
> How about
>
> if ($#ARGV < 0) {
> print "usage: $P [options] patchfile\n";
> print "version: $V\n";
> print "options: -q => quiet\n";
> print " --no-tree => run without a kernel tree\n";
> print " --terse => one line per report\n";
> print " --emacs => emacs compile window format\n";
> print " --file => check a source file\n";
> print " --strict => enable more subjective tests\n";
> print " --root => path to the kernel tree root\n";
> + print "When patchfile is -, read standard input.\n";
> exit(1);
> }
>
My ACK.
regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
Signed-off-by: Stefan Richter <[email protected]>
Acked-by: Jiri Slaby <[email protected]>
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
Index: linux/scripts/checkpatch.pl
===================================================================
--- linux.orig/scripts/checkpatch.pl
+++ linux/scripts/checkpatch.pl
@@ -53,6 +53,7 @@ if ($#ARGV < 0) {
print " --file => check a source file\n";
print " --strict => enable more subjective tests\n";
print " --root => path to the kernel tree root\n";
+ print "When patchfile is -, read standard input.\n";
exit(1);
}
--
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/
On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
> Signed-off-by: Stefan Richter <[email protected]>
> Acked-by: Jiri Slaby <[email protected]>
> ---
> scripts/checkpatch.pl | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux/scripts/checkpatch.pl
> ===================================================================
> --- linux.orig/scripts/checkpatch.pl
> +++ linux/scripts/checkpatch.pl
> @@ -53,6 +53,7 @@ if ($#ARGV < 0) {
> print " --file => check a source file\n";
> print " --strict => enable more subjective tests\n";
> print " --root => path to the kernel tree root\n";
> + print "When patchfile is -, read standard input.\n";
> exit(1);
> }
>
Naww .. Why add documentation when you can just make it do the right
thing ..
Daniel
Daniel Walker wrote:
> On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
>> Signed-off-by: Stefan Richter <[email protected]>
>> + print "When patchfile is -, read standard input.\n";
> Naww .. Why add documentation when you can just make it do the right
> thing ..
Why add code for a feature which already exists?
--
Stefan Richter
-=====-==--- ---= -=-==
http://arcgraph.de/sr/
On Fri, 2008-01-11 at 18:29 +0100, Stefan Richter wrote:
> Daniel Walker wrote:
> > On Fri, 2008-01-11 at 18:06 +0100, Stefan Richter wrote:
> >> Signed-off-by: Stefan Richter <[email protected]>
> >> + print "When patchfile is -, read standard input.\n";
>
> > Naww .. Why add documentation when you can just make it do the right
> > thing ..
>
> Why add code for a feature which already exists?
To make it simpler to use .. A good percentage (if not all) command line
unix data processing utils accept piped data directly, without the need
for the "-" .. That's the expect usage .. checkpatch.pl doesn't need to
deviate from the norm, there is no size constraint, there is no line
limit .. So we're not duplicating functionality, we're making the tool
conform ..
Daniel
On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> Signed-off-by: Stefan Richter <[email protected]>
> Acked-by: Jiri Slaby <[email protected]>
As an absolute minimum this seems reasonable to me. I guess we could
make no arguments default to '-' also. There are up and downsides to
doing that, as currently no arguments currently tell you the usage and
with this patch would point clearly out the '-' option. Just assuming
stding would lose easy access to usage, which may actually be more
confusing for the beginner. Hmmm. Cirtainly will include this
documentation change if nothing else.
> ---
> scripts/checkpatch.pl | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux/scripts/checkpatch.pl
> ===================================================================
> --- linux.orig/scripts/checkpatch.pl
> +++ linux/scripts/checkpatch.pl
> @@ -53,6 +53,7 @@ if ($#ARGV < 0) {
> print " --file => check a source file\n";
> print " --strict => enable more subjective tests\n";
> print " --root => path to the kernel tree root\n";
> + print "When patchfile is -, read standard input.\n";
> exit(1);
> }
-apw
On Mon, 2008-01-14 at 17:17 +0000, Andy Whitcroft wrote:
> On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> > Signed-off-by: Stefan Richter <[email protected]>
> > Acked-by: Jiri Slaby <[email protected]>
>
> As an absolute minimum this seems reasonable to me. I guess we could
> make no arguments default to '-' also. There are up and downsides to
> doing that, as currently no arguments currently tell you the usage and
> with this patch would point clearly out the '-' option. Just assuming
> stding would lose easy access to usage, which may actually be more
> confusing for the beginner. Hmmm. Cirtainly will include this
> documentation change if nothing else.
>
The patch that I submitted checks STDIN for piped data, and if there is
any it will default to checking that incoming data .. That's regardless
of the number of arguments given ..
Daniel
On Mon, Jan 14, 2008 at 09:35:15AM -0800, Daniel Walker wrote:
>
> On Mon, 2008-01-14 at 17:17 +0000, Andy Whitcroft wrote:
> > On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote:
> > > Signed-off-by: Stefan Richter <[email protected]>
> > > Acked-by: Jiri Slaby <[email protected]>
> >
> > As an absolute minimum this seems reasonable to me. I guess we could
> > make no arguments default to '-' also. There are up and downsides to
> > doing that, as currently no arguments currently tell you the usage and
> > with this patch would point clearly out the '-' option. Just assuming
> > stding would lose easy access to usage, which may actually be more
> > confusing for the beginner. Hmmm. Cirtainly will include this
> > documentation change if nothing else.
> >
>
> The patch that I submitted checks STDIN for piped data, and if there is
> any it will default to checking that incoming data .. That's regardless
> of the number of arguments given ..
So it does, however that of itself differs from the unix norm; as with
this I cannot run checkpatch and "type" (ie paste) a patch fragment to
check it. So I don't think we want the semantics as you have there,
as its confusing to the experienced user and inconsistent with the norm.
Either we should document the standard '-' usage as has been suggested
elsewhere or always assume stdin with no parameters.
-apw
On Mon, 2008-01-14 at 19:12 +0000, Andy Whitcroft wrote:
> So it does, however that of itself differs from the unix norm; as with
> this I cannot run checkpatch and "type" (ie paste) a patch fragment to
> check it. So I don't think we want the semantics as you have there,
> as its confusing to the experienced user and inconsistent with the
> norm.
> Either we should document the standard '-' usage as has been suggested
> elsewhere or always assume stdin with no parameters.
I'm not sure I understand what you mean .. Can you give an example?
Daniel
Daniel Walker wrote:
> On Mon, 2008-01-14 at 19:12 +0000, Andy Whitcroft wrote:
>> with this I cannot run checkpatch and "type" (ie paste) a patch
>> fragment to check it.
...
> I'm not sure I understand what you mean .. Can you give an example?
AFAIU Daniel's patch still leaves the possibility to use the '-' syntax,
doesn't it?
(The program 'cat' is of the kind which always reads from stdin if no
file name is given, or if '-' is given instead of a file name. So,
'cat' allows Andy to start it and then type something in for cat to
process in either case, while checkpatch supports this only with '-' as
argument. OTOH it's easier to get checkpatch to print a usage note than
it is with cat. There you have to type no less than 'cat --help', or at
least with GNU cat.)
--
Stefan Richter
-=====-==--- ---= -===-
http://arcgraph.de/sr/
On 01/14/2008 08:31 PM, Stefan Richter wrote:
> it is with cat. There you have to type no less than 'cat --help', or at
> least with GNU cat.)
--h (--he --hel --help) suffices due to getopt_long gnu implementation
On Mon, 2008-01-14 at 20:31 +0100, Stefan Richter wrote:
> AFAIU Daniel's patch still leaves the possibility to use the '-'
> syntax,
> doesn't it?
>
> (The program 'cat' is of the kind which always reads from stdin if no
> file name is given, or if '-' is given instead of a file name. So,
> 'cat' allows Andy to start it and then type something in for cat to
> process in either case, while checkpatch supports this only with '-'
> as
> argument. OTOH it's easier to get checkpatch to print a usage note
> than
> it is with cat. There you have to type no less than 'cat --help', or
> at
> least with GNU cat.)
I tried the following with my changes,
./scripts/checkpatch.pl -
cat | ./scripts/checkpatch.pl
Both allowed pasting patches and pressing Ctrl-D , then it processed the
patch..
and
cat <patchname> | ./scripts/checkpatch.pl
Also processed the patch ..
Daniel