* [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@selenic.com, 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@selenic.com, 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