| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1433278159.4861.99.camel@perches.com>
Date: Tue, 02 Jun 2015 13:49:19 -0700
From: Joe Perches <joe@...ches.com>
To: Petr Mladek <pmladek@...e.cz>
Cc: Andy Whitcroft <apw@...onical.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] checkpatch: Make the output better readable
On Tue, 2015-06-02 at 12:44 +0200, Petr Mladek wrote:
> On Tue 2015-06-02 02:52:03, Joe Perches wrote:
> > On Tue, 2015-06-02 at 11:13 +0200, Petr Mladek wrote:
> > > On Mon 2015-06-01 08:50:24, Joe Perches wrote:
> > > > On Mon, 2015-06-01 at 16:25 +0200, Petr Mladek wrote:
> > > > > I always have troubles to parse checkpatch.pl output when I check
> > > > > the whole patchset. It is hard to say which messages belongs to
> > > > > what patch.
> > > > >
> > > > > This patch does few small changes to make the output look better
> > > > > for me:
> > > > >
> > > > > + delimit each patch from each other with dashes and empty line
> > > > > + remove empty line after the summary
> > > >
> > > > I've no objection about this, but don't much care either.
> > > >
> > > > > + print message about false positives only once
> > > >
> > > > This bit seems sensible, thanks.
> > > >
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -720,8 +720,14 @@ my @fixed_deleted = ();
> > > > > my $fixlinenr = -1;
> > > > >
> > > > > my $vname;
> > > > > +my $filenum = 0;
> > > > > for my $filename (@ARGV) {
> > > > > my $FILE;
> > > > > +
> > > > > + if ($filenum++ && $quiet == 0) {
> > > > > + print "--------------------------------------------------------------------------------\n";
> > > >
> > > > Perhaps more perlish would be print '-' x 81 . '\n\n';
> > > > Dunno why you chose 81 though, it seems an unusual number.
> > >
> > > Are you sure, please? I have just counted it again and I see 80
> > > dashes. Is it possible that you counted the initial quotation
> > > mark '"'?
> >
> > My mistake, I neglected to account for the cr in echo|wc
> >
> > > Well, I do not mind about the number of dashes. Feel free to update
> > > it in case you merge it.
> >
> > I don't actually merge stuff, I can forward it to
> > Andrew Morton though,
>
> That would be nice. Thanks in advance.
>
> > but perhaps it'd be better to
> > check $#ARGV > 1 and emit something like
> > "$filename is being processed\n"
> > so that there is a delimiter before and after each file
>
> I personally do not like this idea much. It would create another
> long line and kind of hide the warnings and errors. IMHO, the dashes
> are better and enough. But I am not UI guy.
>
> But feel free to improve it as you like.
>
> > Another option for you is to add --emacs on the command line.
> > That prefixes patch filename & location before each message.
>
> Thanks for the hint. I was not aware of it. Well, it still looks
> messy without my patch.
Maybe this:
If there are multiple patches/files on the command line,
use a prefix before the patch/file message output like:
--------------
patch/filename
--------------
to make the identifying which messages go with which
file/patch a bit easier to parse.
Move the perl version and false positive messages after
all the files have been scanned so that they are emitted
only once.
Standardize the NOTE: <...> form to always emit a blank
line before the NOTE and always use print << "EOM" style.
---
scripts/checkpatch.pl | 62 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c8032a0..eaa76bd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -197,11 +197,11 @@ sub hash_show_words {
my ($hashRef, $prefix) = @_;
if ($quiet == 0 && keys %$hashRef) {
- print "NOTE: $prefix message types:";
+ print "\nNOTE: $prefix message types:";
foreach my $word (sort keys %$hashRef) {
print " $word";
}
- print "\n\n";
+ print "\n";
}
}
@@ -741,6 +741,13 @@ for my $filename (@ARGV) {
push(@rawlines, $_);
}
close($FILE);
+
+ if ($#ARGV > 0 && $quiet == 0) {
+ print '-' x length($vname) . "\n";
+ print "$vname\n";
+ print '-' x length($vname) . "\n";
+ }
+
if (!process($filename)) {
$exit = 1;
}
@@ -755,6 +762,23 @@ for my $filename (@ARGV) {
build_types();
}
+if (!$quiet) {
+ if ($^V lt 5.10.0) {
+ print << "EOM"
+
+NOTE: perl $^V is not modern enough to detect all possible issues.
+ An upgrade to at least perl v5.10.0 is suggested.
+EOM
+ }
+ if ($exit) {
+ print << "EOM"
+
+NOTE: If any of the errors are false positives, please report
+ them to the maintainer, see CHECKPATCH in MAINTAINERS.
+EOM
+ }
+}
+
exit($exit);
sub top_of_kernel_tree {
@@ -5578,22 +5602,18 @@ sub process {
print "total: $cnt_error errors, $cnt_warn warnings, " .
(($check)? "$cnt_chk checks, " : "") .
"$cnt_lines lines checked\n";
- print "\n" if ($quiet == 0);
}
if ($quiet == 0) {
-
- if ($^V lt 5.10.0) {
- print("NOTE: perl $^V is not modern enough to detect all possible issues.\n");
- print("An upgrade to at least perl v5.10.0 is suggested.\n\n");
- }
-
# If there were whitespace errors which cleanpatch can fix
# then suggest that.
if ($rpt_cleaners) {
- print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n";
- print " scripts/cleanfile\n\n";
$rpt_cleaners = 0;
+ print << "EOM"
+
+NOTE: Whitespace errors detected.
+ You may wish to use scripts/cleanpatch or scripts/cleanfile
+EOM
}
}
@@ -5627,6 +5647,7 @@ sub process {
if (!$quiet) {
print << "EOM";
+
Wrote EXPERIMENTAL --fix correction(s) to '$newfile'
Do _NOT_ trust the results written to this file.
@@ -5634,22 +5655,17 @@ Do _NOT_ submit these changes without inspecting them for correctness.
This EXPERIMENTAL file is simply a convenience to help rewrite patches.
No warranties, expressed or implied...
-
EOM
}
}
- if ($clean == 1 && $quiet == 0) {
- print "$vname has no obvious style problems and is ready for submission.\n"
- }
- if ($clean == 0 && $quiet == 0) {
- print << "EOM";
-$vname has style problems, please review.
-
-If any of these errors are false positives, please report
-them to the maintainer, see CHECKPATCH in MAINTAINERS.
-EOM
+ if ($quiet == 0) {
+ print "\n";
+ if ($clean == 1) {
+ print "$vname has no obvious style problems and is ready for submission.\n";
+ } else {
+ print "$vname has style problems, please review.\n";
+ }
}
-
return $clean;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists