[PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Joe Perches]

Add 'fallthrough' pseudo-keyword to enable the removal of comments
like '/* fallthrough */'.

Add a script to convert the fallthrough comments.

The script can be run over any single file or treewide.

For instance, a treewide conversion can be done using:

$ git ls-files -- '*.[ch]' | \
  xargs scripts/cvt_style.pl -o --convert=fallthrough

This currently produces:

$ git diff --shortstat
 1839 files changed, 4377 insertions(+), 4698 deletions(-)

Example fallthrough conversion produced by the script:

$ scripts/cvt_style.pl -o --convert=fallthrough arch/arm/mm/alignment.c

a/arch/arm/mm/alignment.c
b/arch/arm/mm/alignment.c
@@ -695,8 +695,7 @@ thumb2arm(u16 tinstr)
 			return subset[(L<<1) | ((tinstr & (1<<8)) >> 8)] |
 			    (tinstr & 255);		/* register_list */
 		}
-		/* Else, fall through - for illegal instruction case */
-
+		fallthrough;	/* for illegal instruction case */
 	default:
 		return BAD_INSTR;
 	}
@@ -751,8 +750,7 @@ do_alignment_t32_to_handler(unsigned long *pinstr, struct pt_regs *regs,
 	case 0xe8e0:
 	case 0xe9e0:
 		poffset->un = (tinst2 & 0xff) << 2;
-		/* Fall through */
-
+		fallthrough;
 	case 0xe940:
 	case 0xe9c0:
 		return do_alignment_ldrdstrd;

Joe Perches (4):
  net: sctp: Rename fallthrough label to unhandled
  compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use
  Documentation/process: Add fallthrough pseudo-keyword
  scripts/cvt_style.pl: Tool to reformat sources in various ways

 Documentation/process/coding-style.rst |   2 +-
 Documentation/process/deprecated.rst   |  33 +-
 include/linux/compiler_attributes.h    |  17 +
 net/sctp/sm_make_chunk.c               |  12 +-
 scripts/cvt_style.pl                   | 808 +++++++++++++++++++++++++++++++++
 5 files changed, 855 insertions(+), 17 deletions(-)
 create mode 100755 scripts/cvt_style.pl

-- 
2.15.0

[PATCH 1/4] net: sctp: Rename fallthrough label to unhandled

[Joe Perches]

fallthrough may become a pseudo reserved keyword so this only use of
fallthrough is better renamed to allow it.

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/sctp/sm_make_chunk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index e41ed2e0ae7d..48d63956a68c 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2155,7 +2155,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 	case SCTP_PARAM_SET_PRIMARY:
 		if (ep->asconf_enable)
 			break;
-		goto fallthrough;
+		goto unhandled;
 
 	case SCTP_PARAM_HOST_NAME_ADDRESS:
 		/* Tell the peer, we won't support this param.  */
@@ -2166,11 +2166,11 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 	case SCTP_PARAM_FWD_TSN_SUPPORT:
 		if (ep->prsctp_enable)
 			break;
-		goto fallthrough;
+		goto unhandled;
 
 	case SCTP_PARAM_RANDOM:
 		if (!ep->auth_enable)
-			goto fallthrough;
+			goto unhandled;
 
 		/* SCTP-AUTH: Secion 6.1
 		 * If the random number is not 32 byte long the association
@@ -2187,7 +2187,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 
 	case SCTP_PARAM_CHUNKS:
 		if (!ep->auth_enable)
-			goto fallthrough;
+			goto unhandled;
 
 		/* SCTP-AUTH: Section 3.2
 		 * The CHUNKS parameter MUST be included once in the INIT or
@@ -2203,7 +2203,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 
 	case SCTP_PARAM_HMAC_ALGO:
 		if (!ep->auth_enable)
-			goto fallthrough;
+			goto unhandled;
 
 		hmacs = (struct sctp_hmac_algo_param *)param.p;
 		n_elt = (ntohs(param.p->length) -
@@ -2226,7 +2226,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 			retval = SCTP_IERROR_ABORT;
 		}
 		break;
-fallthrough:
+unhandled:
 	default:
 		pr_debug("%s: unrecognized param:%d for chunk:%d\n",
 			 __func__, ntohs(param.p->type), cid);
-- 
2.15.0

Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled

[Nick Desaulniers]

On Sat, Oct 5, 2019 at 9:46 AM Joe Perches <joe@perches.com> wrote:
>
> fallthrough may become a pseudo reserved keyword so this only use of
> fallthrough is better renamed to allow it.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/sctp/sm_make_chunk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index e41ed2e0ae7d..48d63956a68c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2155,7 +2155,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>         case SCTP_PARAM_SET_PRIMARY:
>                 if (ep->asconf_enable)
>                         break;
> -               goto fallthrough;
> +               goto unhandled;
>
>         case SCTP_PARAM_HOST_NAME_ADDRESS:
>                 /* Tell the peer, we won't support this param.  */
> @@ -2166,11 +2166,11 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>         case SCTP_PARAM_FWD_TSN_SUPPORT:
>                 if (ep->prsctp_enable)
>                         break;
> -               goto fallthrough;
> +               goto unhandled;
>
>         case SCTP_PARAM_RANDOM:
>                 if (!ep->auth_enable)
> -                       goto fallthrough;
> +                       goto unhandled;
>
>                 /* SCTP-AUTH: Secion 6.1
>                  * If the random number is not 32 byte long the association
> @@ -2187,7 +2187,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>
>         case SCTP_PARAM_CHUNKS:
>                 if (!ep->auth_enable)
> -                       goto fallthrough;
> +                       goto unhandled;
>
>                 /* SCTP-AUTH: Section 3.2
>                  * The CHUNKS parameter MUST be included once in the INIT or
> @@ -2203,7 +2203,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>
>         case SCTP_PARAM_HMAC_ALGO:
>                 if (!ep->auth_enable)
> -                       goto fallthrough;
> +                       goto unhandled;
>
>                 hmacs = (struct sctp_hmac_algo_param *)param.p;
>                 n_elt = (ntohs(param.p->length) -
> @@ -2226,7 +2226,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>                         retval = SCTP_IERROR_ABORT;
>                 }
>                 break;
> -fallthrough:
> +unhandled:
>         default:

Interesting control flow (goto from one case to the default case, not
sure "fallthrough" was ever the right word for that).  Thanks for the
patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>                 pr_debug("%s: unrecognized param:%d for chunk:%d\n",
>                          __func__, ntohs(param.p->type), cid);
> --
> 2.15.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled

[Kees Cook]

On Sat, Oct 05, 2019 at 09:46:41AM -0700, Joe Perches wrote:
> fallthrough may become a pseudo reserved keyword so this only use of
> fallthrough is better renamed to allow it.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  net/sctp/sm_make_chunk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index e41ed2e0ae7d..48d63956a68c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2155,7 +2155,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  	case SCTP_PARAM_SET_PRIMARY:
>  		if (ep->asconf_enable)
>  			break;
> -		goto fallthrough;
> +		goto unhandled;
>  
>  	case SCTP_PARAM_HOST_NAME_ADDRESS:
>  		/* Tell the peer, we won't support this param.  */
> @@ -2166,11 +2166,11 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  	case SCTP_PARAM_FWD_TSN_SUPPORT:
>  		if (ep->prsctp_enable)
>  			break;
> -		goto fallthrough;
> +		goto unhandled;
>  
>  	case SCTP_PARAM_RANDOM:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		/* SCTP-AUTH: Secion 6.1
>  		 * If the random number is not 32 byte long the association
> @@ -2187,7 +2187,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  
>  	case SCTP_PARAM_CHUNKS:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		/* SCTP-AUTH: Section 3.2
>  		 * The CHUNKS parameter MUST be included once in the INIT or
> @@ -2203,7 +2203,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  
>  	case SCTP_PARAM_HMAC_ALGO:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		hmacs = (struct sctp_hmac_algo_param *)param.p;
>  		n_elt = (ntohs(param.p->length) -
> @@ -2226,7 +2226,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  			retval = SCTP_IERROR_ABORT;
>  		}
>  		break;
> -fallthrough:
> +unhandled:
>  	default:
>  		pr_debug("%s: unrecognized param:%d for chunk:%d\n",
>  			 __func__, ntohs(param.p->type), cid);
> -- 
> 2.15.0
> 

-- 
Kees Cook

Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled

[Neil Horman]

On Sat, Oct 05, 2019 at 09:46:41AM -0700, Joe Perches wrote:
> fallthrough may become a pseudo reserved keyword so this only use of
> fallthrough is better renamed to allow it.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/sctp/sm_make_chunk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index e41ed2e0ae7d..48d63956a68c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2155,7 +2155,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  	case SCTP_PARAM_SET_PRIMARY:
>  		if (ep->asconf_enable)
>  			break;
> -		goto fallthrough;
> +		goto unhandled;
>  
>  	case SCTP_PARAM_HOST_NAME_ADDRESS:
>  		/* Tell the peer, we won't support this param.  */
> @@ -2166,11 +2166,11 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  	case SCTP_PARAM_FWD_TSN_SUPPORT:
>  		if (ep->prsctp_enable)
>  			break;
> -		goto fallthrough;
> +		goto unhandled;
>  
>  	case SCTP_PARAM_RANDOM:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		/* SCTP-AUTH: Secion 6.1
>  		 * If the random number is not 32 byte long the association
> @@ -2187,7 +2187,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  
>  	case SCTP_PARAM_CHUNKS:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		/* SCTP-AUTH: Section 3.2
>  		 * The CHUNKS parameter MUST be included once in the INIT or
> @@ -2203,7 +2203,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  
>  	case SCTP_PARAM_HMAC_ALGO:
>  		if (!ep->auth_enable)
> -			goto fallthrough;
> +			goto unhandled;
>  
>  		hmacs = (struct sctp_hmac_algo_param *)param.p;
>  		n_elt = (ntohs(param.p->length) -
> @@ -2226,7 +2226,7 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
>  			retval = SCTP_IERROR_ABORT;
>  		}
>  		break;
> -fallthrough:
> +unhandled:
>  	default:
>  		pr_debug("%s: unrecognized param:%d for chunk:%d\n",
>  			 __func__, ntohs(param.p->type), cid);
> -- 
> 2.15.0
> 
> 
I'm still not a fan of the pseudo keyword fallthrough, but I don't have
a problem in renaming the label, so

Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Linus Torvalds]

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

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Joe Perches]

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
+}
-- 
2.15.0

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Linus Torvalds]

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

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Joe Perches]

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?

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Miguel Ojeda]

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

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Kees Cook]

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?

-- 
Kees Cook

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

[Miguel Ojeda]

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