* [PATCH] checkpatch: Emit a warning on embedded filenames
@ 2020-10-01 18:28 Joe Perches
2020-10-01 18:47 ` external tool to remove " Joe Perches
2020-10-02 22:18 ` [PATCH] checkpatch: Emit a warning on " Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Joe Perches @ 2020-10-01 18:28 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, LKML
Embedding the complete filename path inside the file
isn't particularly useful as often the path is moved
around and becomes incorrect.
Emit a warning when the source contains the filename.
Signed-off-by: Joe Perches <joe@perches.com>
---
scripts/checkpatch.pl | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a213cdb82ab0..84d2ee118c55 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3273,6 +3273,12 @@ sub process {
}
}
+# check for embedded filenames
+ if ($rawline =~ /^\+.*\Q$realfile\E/) { di
+ WARN("EMBEDDED_FILENAME",
+ "It's generally not useful to have the filename in the file\n" . $herecurr);
+ }
+
# check we are in a valid source file if not then ignore this hunk
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* external tool to remove embedded filenames
2020-10-01 18:28 [PATCH] checkpatch: Emit a warning on embedded filenames Joe Perches
@ 2020-10-01 18:47 ` Joe Perches
2020-10-02 14:49 ` Bhaskar Chowdhury
2020-10-02 22:18 ` [PATCH] checkpatch: Emit a warning on " Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-10-01 18:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, LKML, kernel-mentors, kernelnewbies
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
It's rather unnecessary for files to contain their
path/filename in source code comments.
Here's a trivial little script that can remove
embedded filenames in c90 style comments from files.
This requires git.
It does the following types of removals:
remove individual lines like /* filename */ completely
remove filename from /* filename -- comment */, leave /* comment */
remove filename and any trailing ' *\n' from /* filename, leave /*
remove filename from /* filename, leave /*
remove filename from continuation ' * filename -- comment' leave ' * comment'
remove filename and any trailing ' *\n' from continuation ' * filename\n *\n'
It seems to work well enough.
It does not handle c99 comments.
No // filename variants are removed.
Running it on today's -next gives:
$ perl remove_embedded_filenames.pl
$ git diff --shortstat
2310 files changed, 354 insertions(+), 4239 deletions(-)
It's also possible to give any filename or path
as an argument to the script
For instance:
$ perl remove_embedded_filenames.pl drivers/net
[-- Attachment #2: remove_embedded_filenames.pl --]
[-- Type: application/x-perl, Size: 2615 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: external tool to remove embedded filenames
2020-10-01 18:47 ` external tool to remove " Joe Perches
@ 2020-10-02 14:49 ` Bhaskar Chowdhury
2020-10-02 15:13 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Bhaskar Chowdhury @ 2020-10-02 14:49 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Andy Whitcroft, LKML, kernel-mentors, kernelnewbies
[-- Attachment #1: Type: text/plain, Size: 4192 bytes --]
On 11:47 Thu 01 Oct 2020, Joe Perches wrote:
>It's rather unnecessary for files to contain their
>path/filename in source code comments.
>
>Here's a trivial little script that can remove
>embedded filenames in c90 style comments from files.
>
>This requires git.
>
>It does the following types of removals:
>
>remove individual lines like /* filename */ completely
>remove filename from /* filename -- comment */, leave /* comment */
>remove filename and any trailing ' *\n' from /* filename, leave /*
>remove filename from /* filename, leave /*
>remove filename from continuation ' * filename -- comment' leave ' * comment'
>remove filename and any trailing ' *\n' from continuation ' * filename\n *\n'
>
>It seems to work well enough.
>
>It does not handle c99 comments.
>No // filename variants are removed.
>
>Running it on today's -next gives:
>
>$ perl remove_embedded_filenames.pl
>$ git diff --shortstat
> 2310 files changed, 354 insertions(+), 4239 deletions(-)
>
>It's also possible to give any filename or path
>as an argument to the script
>
>For instance:
>
>$ perl remove_embedded_filenames.pl drivers/net
>
>#!/usr/bin/perl -w
>
># script to remove * <filename> comments;
># use: perl remove_embedded_filenames.pl <paths|files>
># e.g.: perl remove_embedded_filenames.pl drivers/net/ethernet/intel
>
>use strict;
>
>my $P = $0;
>my $modified = 0;
>my $quiet = 0;
>
>sub expand_tabs {
> my ($str) = @_;
>
> my $res = '';
> my $n = 0;
> for my $c (split(//, $str)) {
> if ($c eq "\t") {
> $res .= ' ';
> $n++;
> for (; ($n % 8) != 0; $n++) {
> $res .= ' ';
> }
> next;
> }
> $res .= $c;
> $n++;
> }
>
> return $res;
>}
>
>my $args = join(" ", @ARGV);
>my $output = `git ls-files -- $args`;
>my @files = split("\n", $output);
>
>foreach my $file (@files) {
> my $f;
> my $cvt = 0;
> my $text;
>
># read the file
>
> next if ((-d $file));
>
> open($f, '<', $file)
> or die "$P: Can't open $file for read\n";
> $text = do { local($/) ; <$f> };
> close($f);
>
> next if ($text eq "");
>
># Remove the embedded filenames
>
> # remove individual lines like /* filename */ completely
> $cvt += $text =~ s@/\*[ \t]+(?:linux\/)?\Q$file\E[ \t]*\*/[ \t]*\n@@g;
> pos($text) = 0;
> # remove filenamee from /* filename -- comment */, leave /* comment */
> $cvt += $text =~ s@/\*([ \t]+)(?:linux\/)?\Q$file\E[ \t]*[:-]+[ \t]*@/*$1@g;
> pos($text) = 0;
> # remove filename and any trailing ' *\n' from /* filename, leave /*
> $cvt += $text =~ s@/\*([ \t]+)(?:linux\/)?\Q$file\E[ \t]*\n([ \t]*\*[ \t]*\n)*(?:[ \t]*\*)?@/*@g;
> pos($text) = 0;
> # remove filename from /* filename, leave /*
> $cvt += $text =~ s@/\*([ \t]+)(?:linux\/)?\Q$file\E[ \t]*\n@/*@g;
> pos($text) = 0;
> # remove filename from continuation ' * filename -- comment'
> # leave ' * comment'
> $cvt += $text =~ s/([ \t]+)\*([ \t]*)(?:linux\/)?\Q$file\E[ \t]*[:-]+[ \t]*/$1*$2/g;
> pos($text) = 0;
> # remove filename and any trailing ' *\n' from
> # continuation ' * filename\n *\n'
> $cvt += $text =~ s/([ \t]*)\*([ \t]*)(?:linux\/)?\Q$file\E[ \t]*\n([ \t]*\*[ \t]*\n)*//g;
> pos($text) = 0;
>
># write the file if something was changed
>
> if ($cvt > 0) {
> $modified = 1;
> print("$file\n");
> open($f, '>', $file)
> or die "$P: Can't open $file for write\n";
> print $f $text;
> close($f);
> }
>}
>
>if ($modified && !$quiet) {
> print <<EOT;
>
>Warning: these changes may not be correct.
>
>These changes should be carefully reviewed manually and not combined with
>any functional changes.
>
>Compile, build and test your changes.
>
>You should understand and be responsible for all object changes.
>
>Make sure you read Documentation/SubmittingPatches before sending
>any changes to reviewers, maintainers or mailing lists.
>EOT
>}
Joe,
Suggestion.... please take those damn EOT lines out of it ..absolutely not
required...or did you put for your own purpose?? As I believe it not the final
product. Anyway, it would be good if those not there.
Yup, I do like the "individual option" stuff ...so, you can only mess around
single thing than the whole lot.
~Bhaskar
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: external tool to remove embedded filenames
2020-10-02 14:49 ` Bhaskar Chowdhury
@ 2020-10-02 15:13 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-10-02 15:13 UTC (permalink / raw)
To: Bhaskar Chowdhury; +Cc: Andrew Morton, Andy Whitcroft, LKML, kernelnewbies
( * removing mentors, I originally meant to send that to mentees )
On Fri, 2020-10-02 at 20:19 +0530, Bhaskar Chowdhury wrote:
> On 11:47 Thu 01 Oct 2020, Joe Perches wrote:
> > It's rather unnecessary for files to contain their
> > path/filename in source code comments.
> >
> > Here's a trivial little script that can remove
> > embedded filenames in c90 style comments from files.
> >
> > This requires git.
[]
> > Running it on today's -next gives:
> >
> > $ perl remove_embedded_filenames.pl
> > $ git diff --shortstat
> > 2310 files changed, 354 insertions(+), 4239 deletions(-)
> >
> > It's also possible to give any filename or path
> > as an argument to the script
> >
> > For instance:
> >
> > $ perl remove_embedded_filenames.pl drivers/net
The below was an attachment, it's odd that your
mailer quoted it.
> > #!/usr/bin/perl -w
[]
> > if ($modified && !$quiet) {
> > print <<EOT;
> > Warning: these changes may not be correct.
> >
> > These changes should be carefully reviewed manually and not combined with
> > any functional changes.
> >
> > Compile, build and test your changes.
> >
> > You should understand and be responsible for all object changes.
> >
> > Make sure you read Documentation/SubmittingPatches before sending
> > any changes to reviewers, maintainers or mailing lists.
> > EOT
> > }
>
> Suggestion.... please take those damn EOT lines out of it
No. What's your actual problem with it?
It's a tool and it may not be perfect.
It merely emits a single message if it removes
filenames from files.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Emit a warning on embedded filenames
2020-10-01 18:28 [PATCH] checkpatch: Emit a warning on embedded filenames Joe Perches
2020-10-01 18:47 ` external tool to remove " Joe Perches
@ 2020-10-02 22:18 ` Andrew Morton
2020-10-02 23:35 ` Joe Perches
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2020-10-02 22:18 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, LKML
On Thu, 01 Oct 2020 11:28:10 -0700 Joe Perches <joe@perches.com> wrote:
> Embedding the complete filename path inside the file
> isn't particularly useful as often the path is moved
> around and becomes incorrect.
>
> Emit a warning when the source contains the filename.
>
> ...
>
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3273,6 +3273,12 @@ sub process {
> }
> }
>
> +# check for embedded filenames
> + if ($rawline =~ /^\+.*\Q$realfile\E/) { di
> + WARN("EMBEDDED_FILENAME",
> + "It's generally not useful to have the filename in the file\n" . $herecurr);
> + }
> +
I removed that " di". Please check that I merged the correct version
of this!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Emit a warning on embedded filenames
2020-10-02 22:18 ` [PATCH] checkpatch: Emit a warning on " Andrew Morton
@ 2020-10-02 23:35 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-10-02 23:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, LKML
On Fri, 2020-10-02 at 15:18 -0700, Andrew Morton wrote:
> On Thu, 01 Oct 2020 11:28:10 -0700 Joe Perches <joe@perches.com> wrote:
>
> > Embedding the complete filename path inside the file
> > isn't particularly useful as often the path is moved
> > around and becomes incorrect.
> >
> > Emit a warning when the source contains the filename.
> >
> > ...
> >
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3273,6 +3273,12 @@ sub process {
> > }
> > }
> >
> > +# check for embedded filenames
> > + if ($rawline =~ /^\+.*\Q$realfile\E/) { di
> > + WARN("EMBEDDED_FILENAME",
> > + "It's generally not useful to have the filename in the file\n" . $herecurr);
> > + }
> > +
>
> I removed that " di". Please check that I merged the correct version
> of this!
Thanks, it must have been added accidentally in my email client.
Combined, the patches are correct.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-02 23:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 18:28 [PATCH] checkpatch: Emit a warning on embedded filenames Joe Perches
2020-10-01 18:47 ` external tool to remove " Joe Perches
2020-10-02 14:49 ` Bhaskar Chowdhury
2020-10-02 15:13 ` Joe Perches
2020-10-02 22:18 ` [PATCH] checkpatch: Emit a warning on " Andrew Morton
2020-10-02 23:35 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).