| Message ID | cover.1570292505.git.joe@perches.com |
|---|---|
| Headers | show |
| Series | treewide: Add 'fallthrough' pseudo-keyword | expand |
On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote: > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > like '/* fallthrough */'. I applied patches 1-3 to my tree just to make it easier for people to start doing this. Maybe some people want to do the conversion on their own subsystem rather than with a global script, but even if not, this looks better as a "prepare for the future" series and I feel that if we're doing the "fallthrough" thing eventually, the sooner we do the prepwork the better. I'm a tiny bit worried that the actual conversion is just going to cause lots of pain for the stable people, but I'll not worry about it _too_ much. If the stable people decide that it's too painful for them to deal with the comment vs keyword model, they may want to backport these three patches too. Linus
On Fri, 2019-10-11 at 09:29 -0700, Linus Torvalds wrote: > On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote: > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > > like '/* fallthrough */'. > > I applied patches 1-3 to my tree just to make it easier for people to > start doing this. Maybe some people want to do the conversion on their > own subsystem rather than with a global script, but even if not, this > looks better as a "prepare for the future" series and I feel that if > we're doing the "fallthrough" thing eventually, the sooner we do the > prepwork the better. > > I'm a tiny bit worried that the actual conversion is just going to > cause lots of pain for the stable people, but I'll not worry about it > _too_ much. If the stable people decide that it's too painful for them > to deal with the comment vs keyword model, they may want to backport > these three patches too. Shouldn't a conversion script be public somewhere? The old cvt_style script could be reduced to something like the below. From: Joe Perches <joe@perches.com> Date: Fri, 11 Oct 2019 10:34:04 -0700 Subject: [PATCH] scripts:cvt_fallthrough.pl: Add script to convert /* fallthrough */ comments Convert /* fallthrough */ style comments to the pseudo-keyword fallthrough; to allow clang 10 and higher to work at finding missing fallthroughs too. Requires a git repository and this overwrites the input sources. Run with a path like: ./scripts/cvt_fallthrough.pl block and all files in the path will be converted or a specific file like: ./scripts/cvt_fallthrough.pl drivers/net/wireless/zydas/zd1201.c Signed-off-by: Joe Perches <joe@perches.com> --- scripts/cvt_fallthrough.pl | 201 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100755 scripts/cvt_fallthrough.pl diff --git a/scripts/cvt_fallthrough.pl b/scripts/cvt_fallthrough.pl new file mode 100755 index 000000000000..013379464f86 --- /dev/null +++ b/scripts/cvt_fallthrough.pl @@ -0,0 +1,201 @@ +#!/usr/bin/perl -w + +# script to modify /* fallthrough */ style comments to fallthrough; +# use: perl cvt_fallthrough.pl <paths|files> +# e.g.: perl cvtfallthrough.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 ""); + + # for style: + + # /* <fallthrough comment> */ + # case FOO: + + # while (comment has fallthrough and next non-blank, non-continuation line is (case or default:)) { + # remove newline, whitespace before, fallthrough comment and whitespace after + # insert newline, adjusted spacing and fallthrough; newline + # } + + my $count = 0; + my @fallthroughs = ( + 'fallthrough', + '@fallthrough@', + 'lint -fallthrough[ \t]*', + '[ \t.!]*(?:ELSE,? |INTENTIONAL(?:LY)? )?', + 'intentional(?:ly)?[ \t]*fall(?:(?:s | |-)[Tt]|t)hr(?:ough|u|ew)', + '(?:else,?\s*)?FALL(?:S | |-)?THR(?:OUGH|U|EW)[ \t.!]*(?:-[^\n\r]*)?', + '[ \t.!]*(?:Else,? |Intentional(?:ly)? )?', + 'Fall(?:(?:s | |-)[Tt]|t)hr(?:ough|u|ew)[ \t.!]*(?:-[^\n\r]*)?', + '[ \t.!]*(?:[Ee]lse,? |[Ii]ntentional(?:ly)? )?', + 'fall(?:s | |-)?thr(?:ough|u|ew)[ \t.!]*(?:-[^\n\r]*)?', + ); + do { + $count = 0; + pos($text) = 0; + foreach my $ft (@fallthroughs) { + my $regex = '(((?:[ \t]*\n)*[ \t]*)(/\*\s*(?!\*/)?\b' . "$ft" . '\s*(?!\*/)?\*/(?:[ \t]*\n)*)([ \t]*))(?:case\s+|default\s*:)'; + + while ($text =~ m{${regex}}i) { + my $all_but_case = $1; + my $space_before = $2; + my $fallthrough = $3; + my $space_after = $4; + my $pos_before = $-[1]; + my $pos_after = $+[3]; + + # try to maintain any additional comment on the same line + my $comment = ""; + if ($regex =~ /\\r/) { + $comment = $fallthrough; + $comment =~ s@^/\*\s*@@; + $comment =~ s@\s*\*/\s*$@@; + $comment =~ s@^\s*(?:intentional(?:ly)?\s+|else,?\s*)?fall[s\-]*\s*thr(?:ough|u|ew)[\s\.\-]*@@i; + $comment =~ s@\s+$@@; + $comment =~ s@\.*$@@; + $comment = "\t/* $comment */" if ($comment ne ""); + } + substr($text, $pos_before, $pos_after - $pos_before, ""); + substr($text, $pos_before, 0, "\n${space_after}\tfallthrough;${comment}\n"); + pos($text) = $pos_before; + $count++; + } + } + $cvt += $count; + } while ($count > 0); + + # Reset the fallthroughs types to a single regex + + @fallthroughs = ( + 'fall(?:(?:s | |-)[Tt]|t)hr(?:ough|u|ew)' + ); + + # Convert the // <fallthrough> style comments followed by case/default: + + do { + $count = 0; + pos($text) = 0; + foreach my $ft (@fallthroughs) { + my $regex = '(((?:[ \t]*\n)*[ \t]*)(//[ \t]*(?!\n)?\b' . "$ft" . '[ \t]*(?!\n)?\n(?:[ \t]*\n)*)([ \t]*))(?:case\s+|default\s*:)'; + while ($text =~ m{${regex}}i) { + my $all_but_case = $1; + my $space_before = $2; + my $fallthrough = $3; + my $space_after = $4; + my $pos_before = $-[1]; + my $pos_after = $+[3]; + + substr($text, $pos_before, $pos_after - $pos_before, ""); + substr($text, $pos_before, 0, "\n${space_after}\tfallthrough;\n"); + pos($text) = $pos_before; + $count++; + } + } + $cvt += $count; + } while ($count > 0); + + # Convert /* fallthrough */ comment macro lines with trailing \ + + do { + $count = 0; + pos($text) = 0; + foreach my $ft (@fallthroughs) { + my $regex = '((?:[ \t]*\\\\\n)*([ \t]*)(/\*[ \t]*\b' . "$ft" . '[ \t]*\*/)([ \t]*))\\\\\n*([ \t]*(?:case\s+|default\s*:))'; + + while ($text =~ m{${regex}}i) { + my $all_but_case = $1; + my $space_before = $2; + my $fallthrough = $3; + my $space_after = $4; + my $pos_before = $-[2]; + my $pos_after = $+[4]; + + my $oldline = "${space_before}${fallthrough}${space_after}"; + my $len = length(expand_tabs($oldline)); + + my $newline = "${space_before}fallthrough;${space_after}"; + $newline .= "\t" while (length(expand_tabs($newline)) < $len); + + substr($text, $pos_before, $pos_after - $pos_before, ""); + substr($text, $pos_before, 0, "$newline"); + pos($text) = $pos_before; + $count++; + } + } + $cvt += $count; + } while ($count > 0); + +# write the file if something was changed + + if ($cvt > 0) { + $modified = 1; + + open($f, '>', $file) + or die "$P: Can't open $file for write\n"; + print $f $text; + close($f); + + print "fallthroughs: $cvt $file\n" if (!$quiet); + } +} + +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 +}
On Fri, Oct 11, 2019 at 10:43 AM Joe Perches <joe@perches.com> wrote: > > Shouldn't a conversion script be public somewhere? I feel the ones that might want to do the conversion on their own are the ones that don't necessarily trust the script. But I don't even know if anybody does want to, I just feel it's an option. Linus
Hi Linus, On Fri, Oct 11, 2019 at 6:30 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote: > > > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > > like '/* fallthrough */'. > > I applied patches 1-3 to my tree just to make it easier for people to > start doing this. Maybe some people want to do the conversion on their > own subsystem rather than with a global script, but even if not, this > looks better as a "prepare for the future" series and I feel that if > we're doing the "fallthrough" thing eventually, the sooner we do the > prepwork the better. > > I'm a tiny bit worried that the actual conversion is just going to > cause lots of pain for the stable people, but I'll not worry about it > _too_ much. If the stable people decide that it's too painful for them > to deal with the comment vs keyword model, they may want to backport > these three patches too. I was waiting for a v2 series to pick it up given we had some pending changes... Cheers, Miguel
On Fri, Oct 11, 2019 at 08:01:42PM +0200, Miguel Ojeda wrote: > Hi Linus, > > On Fri, Oct 11, 2019 at 6:30 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote: > > > > > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > > > like '/* fallthrough */'. > > > > I applied patches 1-3 to my tree just to make it easier for people to > > start doing this. Maybe some people want to do the conversion on their > > own subsystem rather than with a global script, but even if not, this > > looks better as a "prepare for the future" series and I feel that if > > we're doing the "fallthrough" thing eventually, the sooner we do the > > prepwork the better. > > > > I'm a tiny bit worried that the actual conversion is just going to > > cause lots of pain for the stable people, but I'll not worry about it > > _too_ much. If the stable people decide that it's too painful for them > > to deal with the comment vs keyword model, they may want to backport > > these three patches too. > > I was waiting for a v2 series to pick it up given we had some pending changes... Hmpf, looks like it's in torvalds/master now. Miguel, most of the changes were related to the commit log, IIRC, so that ship has sailed. :( Can the stuff in Documentation/ be improved with a follow-up patch, perhaps?
On Sat, Oct 12, 2019 at 12:08 AM Kees Cook <keescook@chromium.org> wrote: > > On Fri, Oct 11, 2019 at 08:01:42PM +0200, Miguel Ojeda wrote: > > Hi Linus, > > > > On Fri, Oct 11, 2019 at 6:30 PM Linus Torvalds > > <torvalds@linux-foundation.org> wrote: > > > > > > On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote: > > > > > > > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > > > > like '/* fallthrough */'. > > > > > > I applied patches 1-3 to my tree just to make it easier for people to > > > start doing this. Maybe some people want to do the conversion on their > > > own subsystem rather than with a global script, but even if not, this > > > looks better as a "prepare for the future" series and I feel that if > > > we're doing the "fallthrough" thing eventually, the sooner we do the > > > prepwork the better. > > > > > > I'm a tiny bit worried that the actual conversion is just going to > > > cause lots of pain for the stable people, but I'll not worry about it > > > _too_ much. If the stable people decide that it's too painful for them > > > to deal with the comment vs keyword model, they may want to backport > > > these three patches too. > > > > I was waiting for a v2 series to pick it up given we had some pending changes... > > Hmpf, looks like it's in torvalds/master now. Miguel, most of the changes > were related to the commit log, IIRC, so that ship has sailed. :( Can the > stuff in Documentation/ be improved with a follow-up patch, perhaps? Linus seems to have applied some of the improvements to the commit messages, but not those to the content (not sure why, since they were also easy ones). But no worries, we will do those later :) Cheers, Miguel
On Fri, 2019-10-11 at 10:46 -0700, Linus Torvalds wrote: > On Fri, Oct 11, 2019 at 10:43 AM Joe Perches <joe@perches.com> wrote: > > Shouldn't a conversion script be public somewhere? > > I feel the ones that might want to do the conversion on their own are > the ones that don't necessarily trust the script. > > But I don't even know if anybody does want to, I just feel it's an option. What's the harm in keeping the script in the tree until it's no longer needed?