[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>

[PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Joe Perches]

Reserve the pseudo keyword 'fallthrough' for the ability to convert the
various case block /* fallthrough */ style comments to appear to be an
actual reserved word with the same gcc case block missing fallthrough
warning capability.

All switch/case blocks now must end in one of:

	break;
	fallthrough;
	goto <label>;
	return [expression];
	continue;

fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
in gcc version 7..

fallthrough devolves to an empty "do {} while (0)" if the compiler
version (any version less than gcc 7) does not support the attribute.

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/compiler_attributes.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 6b318efd8a74..cdf016596659 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -40,6 +40,7 @@
 # define __GCC4_has_attribute___noclone__             1
 # define __GCC4_has_attribute___nonstring__           0
 # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
+# define __GCC4_has_attribute___fallthrough__         0
 #endif
 
 /*
@@ -185,6 +186,22 @@
 # define __noclone
 #endif
 
+/*
+ * Add the pseudo keyword 'fallthrough' so case statement blocks
+ * must end with any of these keywords:
+ *   break;
+ *   fallthrough;
+ *   goto <label>;
+ *   return [expression];
+ *
+ *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
+ */
+#if __has_attribute(__fallthrough__)
+# define fallthrough                    __attribute__((__fallthrough__))
+#else
+# define fallthrough                    do {} while (0)  /* fallthrough */
+#endif
+
 /*
  * Note the missing underscores.
  *
-- 
2.15.0

Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Miguel Ojeda]

Hi Joe,

On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
>
> Reserve the pseudo keyword 'fallthrough' for the ability to convert the
> various case block /* fallthrough */ style comments to appear to be an
> actual reserved word with the same gcc case block missing fallthrough
> warning capability.
>
> All switch/case blocks now must end in one of:
>
>         break;
>         fallthrough;
>         goto <label>;
>         return [expression];
>         continue;
>
> fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
> in gcc version 7..

Nits: double period, missing "r" in fallthough.

> fallthrough devolves to an empty "do {} while (0)" if the compiler
> version (any version less than gcc 7) does not support the attribute.

Perhaps add a short note why (empty statement warnings maybe? I don't
remember them but it was months ago so maybe it changed).

> Signed-off-by: Joe Perches <joe@perches.com>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Please add Dan's Suggested-by and copy the things I wrote in the
commit message when I proposed this:

  https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4

> ---
>  include/linux/compiler_attributes.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 6b318efd8a74..cdf016596659 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -40,6 +40,7 @@
>  # define __GCC4_has_attribute___noclone__             1
>  # define __GCC4_has_attribute___nonstring__           0
>  # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> +# define __GCC4_has_attribute___fallthrough__         0

This goes after __externally_visible__.

>  #endif
>
>  /*
> @@ -185,6 +186,22 @@
>  # define __noclone
>  #endif
>
> +/*
> + * Add the pseudo keyword 'fallthrough' so case statement blocks
> + * must end with any of these keywords:
> + *   break;
> + *   fallthrough;
> + *   goto <label>;
> + *   return [expression];
> + *
> + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes

This also goes after __externally_visible__.

Please add:

  * Optional: only supported since gcc >= 7.1
  * Optional: only supported since clang >= 10
  * Optional: not supported by icc

As well as:

  clang: https://clang.llvm.org/docs/AttributeReference.html#fallthrough

See how I did it in the link above:

  https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4

> + */
> +#if __has_attribute(__fallthrough__)
> +# define fallthrough                    __attribute__((__fallthrough__))
> +#else
> +# define fallthrough                    do {} while (0)  /* fallthrough */
> +#endif
> +
>  /*
>   * Note the missing underscores.
>   *
> --
> 2.15.0
>

Cheers,
Miguel

Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Nick Desaulniers]

On Sat, Oct 5, 2019 at 10:17 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Hi Joe,
>
> On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
> >
> > Reserve the pseudo keyword 'fallthrough' for the ability to convert the

Have we precedent already for "pseudo keywords?"  I kind of like the
double underscore prefix we use for attributes (which this is one of),
or at least making macro's ALLCAPS as some sort of convention.
Otherwise, someone might be confused on seeing `fallthrough` sprinkled
throughout the code without knowing how it works. `__fallthough` or
`FALLTHROUGH` are maybe less surprising (and potentially easier to
grep)?  Sorry if this has already been discussed; from Miguel's link
below it looks like there used to be underscore prefixes before?

> > various case block /* fallthrough */ style comments to appear to be an
> > actual reserved word with the same gcc case block missing fallthrough
> > warning capability.
> >
> > All switch/case blocks now must end in one of:
> >
> >         break;
> >         fallthrough;
> >         goto <label>;
> >         return [expression];
> >         continue;
> >
> > fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
> > in gcc version 7..
>
> Nits: double period, missing "r" in fallthough.
>
> > fallthrough devolves to an empty "do {} while (0)" if the compiler
> > version (any version less than gcc 7) does not support the attribute.
>
> Perhaps add a short note why (empty statement warnings maybe? I don't
> remember them but it was months ago so maybe it changed).
>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Please add Dan's Suggested-by and copy the things I wrote in the
> commit message when I proposed this:
>
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
>
> > ---
> >  include/linux/compiler_attributes.h | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > index 6b318efd8a74..cdf016596659 100644
> > --- a/include/linux/compiler_attributes.h
> > +++ b/include/linux/compiler_attributes.h
> > @@ -40,6 +40,7 @@
> >  # define __GCC4_has_attribute___noclone__             1
> >  # define __GCC4_has_attribute___nonstring__           0
> >  # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> > +# define __GCC4_has_attribute___fallthrough__         0
>
> This goes after __externally_visible__.
>
> >  #endif
> >
> >  /*
> > @@ -185,6 +186,22 @@
> >  # define __noclone
> >  #endif
> >
> > +/*
> > + * Add the pseudo keyword 'fallthrough' so case statement blocks
> > + * must end with any of these keywords:
> > + *   break;
> > + *   fallthrough;
> > + *   goto <label>;
> > + *   return [expression];
> > + *
> > + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
>
> This also goes after __externally_visible__.
>
> Please add:
>
>   * Optional: only supported since gcc >= 7.1
>   * Optional: only supported since clang >= 10
>   * Optional: not supported by icc
>
> As well as:
>
>   clang: https://clang.llvm.org/docs/AttributeReference.html#fallthrough
>
> See how I did it in the link above:
>
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
>
> > + */
> > +#if __has_attribute(__fallthrough__)
> > +# define fallthrough                    __attribute__((__fallthrough__))
> > +#else
> > +# define fallthrough                    do {} while (0)  /* fallthrough */
> > +#endif
> > +
> >  /*
> >   * Note the missing underscores.
> >   *
> > --
> > 2.15.0
> >
>
> Cheers,
> Miguel



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Joe Perches]

On Mon, 2019-10-07 at 11:14 -0700, Nick Desaulniers wrote:
> > On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
> > > Reserve the pseudo keyword 'fallthrough' for the ability to convert the
> 
> Have we precedent already for "pseudo keywords?"

Many.  see bool vs _Bool, u32 vs uint32_t, etc.

> I kind of like the
> double underscore prefix we use for attributes (which this is one of),

Linus apparently (and rightly IMO) prefers
fallthrough over __fallthrough.

https://lore.kernel.org/lkml/201909161516.A68C8239A@keescook/

Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Kees Cook]

On Sat, Oct 05, 2019 at 07:17:36PM +0200, Miguel Ojeda wrote:
> Hi Joe,
> 
> On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
> >
> > Reserve the pseudo keyword 'fallthrough' for the ability to convert the
> > various case block /* fallthrough */ style comments to appear to be an
> > actual reserved word with the same gcc case block missing fallthrough
> > warning capability.
> >
> > All switch/case blocks now must end in one of:
> >
> >         break;
> >         fallthrough;
> >         goto <label>;
> >         return [expression];
> >         continue;
> >
> > fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
> > in gcc version 7..
> 
> Nits: double period, missing "r" in fallthough.
> 
> > fallthrough devolves to an empty "do {} while (0)" if the compiler
> > version (any version less than gcc 7) does not support the attribute.
> 
> Perhaps add a short note why (empty statement warnings maybe? I don't
> remember them but it was months ago so maybe it changed).
> 
> > Signed-off-by: Joe Perches <joe@perches.com>
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Please add Dan's Suggested-by and copy the things I wrote in the
> commit message when I proposed this:
> 
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4

Perhaps just take this patch directly with Miguel's authorship into the
series and tweak the __fallthrough to fallthrough in subject and body.

-Kees

> 
> > ---
> >  include/linux/compiler_attributes.h | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > index 6b318efd8a74..cdf016596659 100644
> > --- a/include/linux/compiler_attributes.h
> > +++ b/include/linux/compiler_attributes.h
> > @@ -40,6 +40,7 @@
> >  # define __GCC4_has_attribute___noclone__             1
> >  # define __GCC4_has_attribute___nonstring__           0
> >  # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> > +# define __GCC4_has_attribute___fallthrough__         0
> 
> This goes after __externally_visible__.
> 
> >  #endif
> >
> >  /*
> > @@ -185,6 +186,22 @@
> >  # define __noclone
> >  #endif
> >
> > +/*
> > + * Add the pseudo keyword 'fallthrough' so case statement blocks
> > + * must end with any of these keywords:
> > + *   break;
> > + *   fallthrough;
> > + *   goto <label>;
> > + *   return [expression];
> > + *
> > + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> 
> This also goes after __externally_visible__.
> 
> Please add:
> 
>   * Optional: only supported since gcc >= 7.1
>   * Optional: only supported since clang >= 10
>   * Optional: not supported by icc
> 
> As well as:
> 
>   clang: https://clang.llvm.org/docs/AttributeReference.html#fallthrough
> 
> See how I did it in the link above:
> 
>   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
> 
> > + */
> > +#if __has_attribute(__fallthrough__)
> > +# define fallthrough                    __attribute__((__fallthrough__))
> > +#else
> > +# define fallthrough                    do {} while (0)  /* fallthrough */
> > +#endif
> > +
> >  /*
> >   * Note the missing underscores.
> >   *
> > --
> > 2.15.0
> >
> 
> Cheers,
> Miguel

-- 
Kees Cook

Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use

[Miguel Ojeda]

On Thu, Oct 10, 2019 at 10:37 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Oct 05, 2019 at 07:17:36PM +0200, Miguel Ojeda wrote:
> > Hi Joe,
> >
> > On Sat, Oct 5, 2019 at 6:46 PM Joe Perches <joe@perches.com> wrote:
> > >
> > > Reserve the pseudo keyword 'fallthrough' for the ability to convert the
> > > various case block /* fallthrough */ style comments to appear to be an
> > > actual reserved word with the same gcc case block missing fallthrough
> > > warning capability.
> > >
> > > All switch/case blocks now must end in one of:
> > >
> > >         break;
> > >         fallthrough;
> > >         goto <label>;
> > >         return [expression];
> > >         continue;
> > >
> > > fallthough is gcc's __attribute__((__fallthrough__)) which was introduced
> > > in gcc version 7..
> >
> > Nits: double period, missing "r" in fallthough.
> >
> > > fallthrough devolves to an empty "do {} while (0)" if the compiler
> > > version (any version less than gcc 7) does not support the attribute.
> >
> > Perhaps add a short note why (empty statement warnings maybe? I don't
> > remember them but it was months ago so maybe it changed).
> >
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > Please add Dan's Suggested-by and copy the things I wrote in the
> > commit message when I proposed this:
> >
> >   https://github.com/ojeda/linux/commit/668f011a2706ea555987e263f609a5deba9c7fc4
>
> Perhaps just take this patch directly with Miguel's authorship into the
> series and tweak the __fallthrough to fallthrough in subject and body.

Since I was going to pick it up, I would have signed-off-by anyway, but yeah ;P

Cheers,
Miguel

[PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword

[Joe Perches]

Describe the fallthrough pseudo-keyword.

Convert the coding-style.rst example to the keyword style.
Add description and links to deprecated.rst.

Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/process/coding-style.rst |  2 +-
 Documentation/process/deprecated.rst   | 33 +++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index f4a2198187f9..ada573b7d703 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -56,7 +56,7 @@ instead of ``double-indenting`` the ``case`` labels.  E.g.:
 	case 'K':
 	case 'k':
 		mem <<= 10;
-		/* fall through */
+		fallthrough;
 	default:
 		break;
 	}
diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 56280e108d5a..a0ffdc8daef3 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -122,14 +122,27 @@ memory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`)
 
 Implicit switch case fall-through
 ---------------------------------
-The C language allows switch cases to "fall through" when
-a "break" statement is missing at the end of a case. This,
-however, introduces ambiguity in the code, as it's not always
-clear if the missing break is intentional or a bug. As there
-have been a long list of flaws `due to missing "break" statements
+The C language allows switch cases to "fall-through" when a "break" statement
+is missing at the end of a case. This, however, introduces ambiguity in the
+code, as it's not always clear if the missing break is intentional or a bug.
+
+As there have been a long list of flaws `due to missing "break" statements
 <https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow
-"implicit fall-through". In order to identify an intentional fall-through
-case, we have adopted the marking used by static analyzers: a comment
-saying `/* Fall through */`. Once the C++17 `__attribute__((fallthrough))`
-is more widely handled by C compilers, static analyzers, and IDEs, we can
-switch to using that instead.
+"implicit fall-through".
+
+In order to identify intentional fall-through cases, we have adopted a
+pseudo-keyword macro 'fallthrough' which expands to gcc's extension
+__attribute__((__fallthrough__)).  `Statement Attributes
+<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_
+
+When the C17/C18  [[fallthrough]] syntax is more commonly supported by
+C compilers, static analyzers, and IDEs, we can switch to using that syntax
+for the macro pseudo-keyword.
+
+All switch/case blocks must end in one of:
+
+	break;
+	fallthrough;
+	continue;
+	goto <label>;
+	return [expression];
-- 
2.15.0

Re: [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword

[Miguel Ojeda]

On Sat, Oct 5, 2019 at 6:47 PM Joe Perches <joe@perches.com> wrote:
>
> diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
> index 56280e108d5a..a0ffdc8daef3 100644
> --- a/Documentation/process/deprecated.rst
> +++ b/Documentation/process/deprecated.rst
> @@ -122,14 +122,27 @@ memory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`)
>
>  Implicit switch case fall-through
>  ---------------------------------
> -The C language allows switch cases to "fall through" when
> -a "break" statement is missing at the end of a case. This,
> -however, introduces ambiguity in the code, as it's not always
> -clear if the missing break is intentional or a bug. As there
> -have been a long list of flaws `due to missing "break" statements
> +The C language allows switch cases to "fall-through" when a "break" statement
> +is missing at the end of a case. This, however, introduces ambiguity in the
> +code, as it's not always clear if the missing break is intentional or a bug.
> +
> +As there have been a long list of flaws `due to missing "break" statements
>  <https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow
> -"implicit fall-through". In order to identify an intentional fall-through
> -case, we have adopted the marking used by static analyzers: a comment
> -saying `/* Fall through */`. Once the C++17 `__attribute__((fallthrough))`
> -is more widely handled by C compilers, static analyzers, and IDEs, we can
> -switch to using that instead.
> +"implicit fall-through".
> +
> +In order to identify intentional fall-through cases, we have adopted a
> +pseudo-keyword macro 'fallthrough' which expands to gcc's extension
> +__attribute__((__fallthrough__)).  `Statement Attributes
> +<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_
> +
> +When the C17/C18  [[fallthrough]] syntax is more commonly supported by

Note that C17/C18 does not have [[fallthrough]]. C++17 introduced it,
as it is mentioned above. I would keep the
__attribute__((fallthrough)) -> [[fallthrough]] change you did,
though, since that is indeed the standard syntax (given the paragraph
references C++17).

I was told by Aaron Ballman (who is proposing them for C) that it is
more or less likely that it becomes standardized in C2x. However, it
is still not added to the draft (other attributes are already,
though). See N2268 and N2269:

    http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf (fallthrough)
    http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2269.pdf
(attributes in general)

Cheers,
Miguel

Re: [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword

[Nick Desaulniers]

On Sat, Oct 5, 2019 at 10:48 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Oct 5, 2019 at 6:47 PM Joe Perches <joe@perches.com> wrote:
> >
> > +When the C17/C18  [[fallthrough]] syntax is more commonly supported by
>
> Note that C17/C18 does not have [[fallthrough]]. C++17 introduced it,
> as it is mentioned above. I would keep the
> __attribute__((fallthrough)) -> [[fallthrough]] change you did,
> though, since that is indeed the standard syntax (given the paragraph
> references C++17).
>
> I was told by Aaron Ballman (who is proposing them for C) that it is
> more or less likely that it becomes standardized in C2x. However, it
> is still not added to the draft (other attributes are already,
> though). See N2268 and N2269:
>
>     http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf (fallthrough)
>     http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2269.pdf
> (attributes in general)
>

Interesting. I think those links might be useful to include, or drop
the section on C++ style attributes outright. Either way:
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

[PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Joe Perches]

Trivial tool to reformat source code in various ways.

This is an old tool that was recently updated to convert /* fallthrough */
style comments to the new pseudo-keyword fallthrough;

Typical command line use is:
    $ perl scripts/cvt_style --convert=fallthrough <file list>

Available conversions:
	all
	printk_to_pr_level
	printk_KERN_DEBUG_to_pr_debug
	dev_printk_to_dev_level
	dev_printk_KERN_DEBUG_to_dev_dbg
	sdev_printk_to_sdev_level
	sdev_printk_KERN_DEBUG_to_sdev_dbg
	coalesce_formats
	cuddle_open_brace
	cuddle_else
	deparenthesize_returns
	space_after_KERN_level
	space_after_if_while_for_switch
	space_after_for_semicolons
	space_after_comma
	space_before_pointer
	space_around_trigraph
	leading_spaces_to_tabs
	coalesce_semicolons
	remove_trailing_whitespace
	remove_whitespace_before_quoted_newline
	remove_whitespace_before_trailing_semicolon
	remove_whitespace_before_bracket
	remove_parenthesis_whitespace
	remove_single_statement_braces
	remove_whitespace_after_cast
	hoist_assigns_from_if
	convert_c99_comments
	remove_private_data_casts
	remove_static_initializations_to_0
	remove_true_false_comparisons
	remove_NULL_comparisons
	remove_trailing_if_semicolons
	network_comments
	remove_switchforwhileif_semicolons
	detab_else_return
	remove_while_while
	fallthrough
Additional conversions which may not work well:
	(enable individually or with --convert=all --broken)
	move_labels_to_column_1
	space_around_logical_tests
	space_around_assign
	space_around_arithmetic
	CamelCase_to_camel_case

Use --convert=(comma separated list)
   ie: --convert=printk_to_pr_level,coalesce_formats

Output file:
  --overwrite => write the changes to the source file
  --suffix => suffix to append to new file (default: .style)

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/cvt_style.pl | 808 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 808 insertions(+)
 create mode 100755 scripts/cvt_style.pl

diff --git a/scripts/cvt_style.pl b/scripts/cvt_style.pl
new file mode 100755
index 000000000000..fcbda0b1c67a
--- /dev/null
+++ b/scripts/cvt_style.pl
@@ -0,0 +1,808 @@
+#!/usr/bin/perl -w
+
+# Change some style elements of a source file
+# An imperfect source code formatter.
+# Might make trivial patches a bit easier.
+#
+# usage: perl scripts/cvt_kernel_style.pl <files>
+#
+# Licensed under the terms of the GNU GPL License version 2
+
+use strict;
+use Getopt::Long qw(:config no_auto_abbrev);
+
+my $P = $0;
+my $V = '0.4';
+
+my $source_indent = 8;
+my $quiet = 0;
+my $stats = 1;
+my $overwrite = 0;
+my $modified = 0;
+my $suffix = ".style";
+my $convert_options = "";
+my $broken = 0;
+
+my @std_options = (
+    "all",
+    "printk_to_pr_level",
+    "printk_KERN_DEBUG_to_pr_debug",
+    "dev_printk_to_dev_level",
+    "dev_printk_KERN_DEBUG_to_dev_dbg",
+    "sdev_printk_to_sdev_level",
+    "sdev_printk_KERN_DEBUG_to_sdev_dbg",
+    "coalesce_formats",
+    "cuddle_open_brace",
+    "cuddle_else",
+    "deparenthesize_returns",
+    "space_after_KERN_level",
+    "space_after_if_while_for_switch",
+    "space_after_for_semicolons",
+    "space_after_comma",
+    "space_before_pointer",
+    "space_around_trigraph",
+    "leading_spaces_to_tabs",
+    "coalesce_semicolons",
+    "remove_trailing_whitespace",
+    "remove_whitespace_before_quoted_newline",
+    "remove_whitespace_before_trailing_semicolon",
+    "remove_whitespace_before_bracket",
+    "remove_parenthesis_whitespace",
+    "remove_single_statement_braces",
+    "remove_whitespace_after_cast",
+    "hoist_assigns_from_if",
+    "convert_c99_comments",
+    "remove_private_data_casts",
+    "remove_static_initializations_to_0",
+    "remove_true_false_comparisons",
+    "remove_NULL_comparisons",
+    "remove_trailing_if_semicolons",
+    "network_comments",
+    "remove_switchforwhileif_semicolons",
+    "detab_else_return",
+    "remove_while_while",
+    "fallthrough",
+);
+
+my @other_options = (
+    "move_labels_to_column_1",
+    "space_around_logical_tests",
+    "space_around_assign",
+    "space_around_arithmetic",
+    "CamelCase_to_camel_case"
+);
+
+my $version = 0;
+my $help = 0;
+
+my $logFunctions = qr{(?x:
+	printk|
+	([a-z0-9_]+)_(debug|dbg|vdbg|devel|info|warn|warning|err|notice|alert|crit|emerg|cont)|
+	WARN|
+	panic
+)};
+
+my $match_balanced_parentheses = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+my $match_balanced_braces      = qr/(\{(?:[^\{\}]++|(?-1))*\})/;
+my $do_cvt;
+
+my %hash;
+
+sub set_all_options {
+    my ($enabled) = @_;
+
+    foreach my $opt (@std_options) {
+	$hash{$opt} = $enabled;
+    }
+
+    if ($broken > 0 || $enabled == -1) {
+	foreach my $opt (@other_options) {
+	    $hash{$opt} = $enabled;
+	}
+    }
+
+}
+
+if (!GetOptions(
+		'source-indent=i' => \$source_indent,
+		'convert=s' => \$convert_options,
+		'broken!' => \$broken,
+		'stats!' => \$stats,
+		'o|overwrite!' => \$overwrite,
+		'q|quiet!' => \$quiet,
+		'v|version' => \$version,
+		'h|help|usage' => \$help,
+		)) {
+    die "$P: invalid argument - use --help if necessary\n";
+}
+
+if ($help) {
+    usage();
+    exit 0;
+}
+
+if ($version) {
+    print "$P: v$V\n";
+    exit 0;
+}
+
+my $max_spaces_before_tab = $source_indent - 1;
+my $spaces_to_tab = sprintf("%*s", $source_indent, "");
+
+set_all_options(-1);
+
+my @actual_options = split(',', $convert_options);
+foreach my $opt (@actual_options) {
+    if ($opt eq "all") {
+	set_all_options(0);
+    }
+    if (exists($hash{$opt})) {
+	$hash{$opt} = 0;
+    } else {
+	print "Invalid --convert option: '$opt', ignored\n";
+    }
+}
+
+sub usage {
+    print <<EOT;
+usage: $P [options] <files>
+version: $V
+
+EOT
+    print "Available conversions:\n";
+    foreach my $convert (@std_options) {
+	print "\t$convert\n";
+    }
+    print "Additional conversions which may not work well:\n";
+    print "\t(enable individually or with --convert=all --broken)\n";
+    foreach my $convert (@other_options) {
+	print "\t$convert\n";
+    }
+    print "\n";
+    print "Use --convert=(comma separated list)\n";
+    print "   ie: --convert=printk_to_pr_level,coalesce_formats\n";
+    print <<EOT;
+
+Input source file descriptions:
+  --source-indent => How many spaces are used for an indent (default: 8)
+
+Output file:
+  --overwrite => write the changes to the source file
+  --suffix => suffix to append to new file (default: .style)
+
+Other options:
+  --quiet => don't show conversion warning messages (default: disabled)
+  --stats => show conversions done (default: enabled)
+  --version => show version
+  --help => show this help information
+EOT
+}
+
+sub check_label {
+    my ($leading, $label) = @_;
+
+    if ($label == "default") {
+	return "$leading$label:";
+    }
+    return "$label:";
+}
+
+sub check_for {
+    my ($leading, $test1, $test2, $test3) = @_;
+
+    $test1 =~ s/^\s+|\s+$//g;
+    $test2 =~ s/^\s+|\s+$//g;
+    $test3 =~ s/^\s+|\s+$//g;
+
+    return "${leading}for ($test1; $test2; $test3)";
+}
+
+sub tabify {
+    my ($leading) = @_;
+
+#convert leading spaces to tabs
+    1 while $leading =~ s@^([\t]*)$spaces_to_tab@$1\t@g;
+#Remove spaces before a tab
+    1 while $leading =~ s@^([\t]*)([ ]{1,$max_spaces_before_tab})\t@$1\t@g;
+
+    return "$leading";
+}
+
+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;
+}
+
+sub default_substitute {
+    my ($argument) = @_;
+
+    return "$argument";
+}
+
+sub subst_line_mode_fn {
+    my ($lines, $match, $fn, $args) = @_;
+
+    my $function = \&$fn;
+    my @lines = split("\n", $lines);
+    my $count = 0;
+
+    foreach my $line (@lines) {
+	my $oldline = $line;
+	$line =~ s@$match@&$function(eval $args)@ge;
+	$count++ if ($oldline ne $line);
+    }
+
+    return ($count, join("\n", @lines) . "\n");
+}
+
+sub subst_line_mode {
+    my ($lines, $match, $substitute) = @_;
+
+    return subst_line_mode_fn($lines, $match, "default_substitute", $substitute);
+}
+
+sub convert {
+    my ($check) = @_;
+
+    return 1 if ($hash{$check} >= 0);
+
+    return 0;
+}
+
+sub strip_leading_paren {
+    my ($string) = @_;
+    $string =~ s@^\(\s*@@g;
+    return $string;
+}
+
+sub strip_outer_paren {
+    my ($string) = @_;
+    $string =~ s@^\(\s*@@g;
+    $string =~ s@\s*\)$@@g;
+    return $string;
+}
+
+sub trim_trail {
+    my ($string) = @_;
+    $string =~ s@\s*$@@g;
+    return $string;
+}
+
+
+foreach my $file (@ARGV) {
+    my $f;
+    my $text;
+    my $oldtext;
+
+# read the file
+
+    next if ((-d $file));
+
+    open($f, '<', $file)
+	or die "$P: Can't open $file for read\n";
+    $oldtext = do { local($/) ; <$f> };
+    close($f);
+
+    next if ($oldtext eq "");
+
+    $text = $oldtext;
+
+# Convert printk(KERN_<level> to pr_<level>(
+    $do_cvt = "printk_to_pr_level";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bprintk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1\(@g;
+	$text =~ s@\bpr_warning\b@pr_warn@g;
+
+	$hash{$do_cvt} += $text =~ s@\bprintk_ratelimited\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1_ratelimited\(@g;
+	$text =~ s@\bpr_warning_ratelimited\b@pr_warn_ratelimited@g;
+
+	$hash{$do_cvt} += $text =~ s@\bprintk_once\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*@pr_\L$1_once\(@g;
+	$text =~ s@\bpr_warning_once\b@pr_warn_once@g;
+    }
+
+# Convert printk(KERN_DEBUG to pr_debug(
+    $do_cvt = "printk_KERN_DEBUG_to_pr_debug";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bprintk\s*\(\s*KERN_(DEBUG)\s*@pr_\L$1\(@g;
+    }
+
+# Convert dev_printk(KERN_<level> to dev_<level>(
+    $do_cvt = "dev_printk_to_dev_level";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bdev_printk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE)\s*,\s*@dev_\L$1\(@g;
+	$text =~ s@\bdev_warning\b@dev_warn@g;
+
+	$hash{$do_cvt} += $text =~ s@\bdev_printk_ratelimited\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*,\s*@dev_\L$1_ratelimited\(@g;
+	$text =~ s@\bdev_warning_ratelimited\b@dev_warn_ratelimited@g;
+
+	$hash{$do_cvt} += $text =~ s@\bdev_printk_once\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*,\s*@dev_\L$1_once\(@g;
+	$text =~ s@\bdev_warning_once\b@dev_warn_once@g;
+    }
+
+# Convert dev_printk(KERN_DEBUG to dev_dbg(
+    $do_cvt = "dev_printk_KERN_DEBUG_to_dev_dbg";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bdev_printk\s*\(\s*KERN_(DEBUG)\s*,\s*@dev_dbg\(@g;
+    }
+
+# Convert sdev_printk(KERN_<level> to sdev_<level>(
+    $do_cvt = "sdev_printk_to_sdev_level";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bsdev_printk\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE)\s*,\s*@sdev_\L$1\(@g;
+	$text =~ s@\bsdev_warning\b@sdev_warn@g;
+
+	$hash{$do_cvt} += $text =~ s@\bsdev_printk_ratelimited\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*,\s*@sdev_\L$1_ratelimited\(@g;
+	$text =~ s@\bsdev_warning_ratelimited\b@sdev_warn_ratelimited@g;
+
+	$hash{$do_cvt} += $text =~ s@\bsdev_printk_once\s*\(\s*KERN_(INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)\s*,\s*@sdev_\L$1_once\(@g;
+	$text =~ s@\bsdev_warning_once\b@sdev_warn_once@g;
+    }
+
+# Convert sdev_printk(KERN_DEBUG to sdev_dbg(
+    $do_cvt = "sdev_printk_KERN_DEBUG_to_sdev_dbg";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\bsdev_printk\s*\(\s*KERN_(DEBUG)\s*,\s*@sdev_dbg\(@g;
+    }
+
+# Coalesce long formats
+    $do_cvt = "coalesce_formats";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@\b(${logFunctions}\s*\([^;]+)(?!\\n)\"\s*\n\s*\"@$1@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Add space between KERN_<LEVEL> and open quote
+    $do_cvt = "space_after_KERN_level";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\b(KERN_(DEBUG|INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT)) \"@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\b(KERN_(DEBUG|INFO|WARNING|ERR|ALERT|CRIT|EMERG|NOTICE|CONT))[ \t]*\"@$1 \"@g;
+    }
+
+# Remove unnecessary parentheses around return
+    $do_cvt = "deparenthesize_returns";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@\breturn\s*${match_balanced_parentheses}\s*;@"return " . scalar(strip_outer_paren($1)) . ";"@egx;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# This doesn't work very well, too many comments modified
+# Put labels (but not "default:") on column 1
+    $do_cvt = "move_labels_to_column_1";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@^([ \t]+)([A-Za-z0-9_]+)\s*:[ \t]*:[ \t]*$@check_label($1, $2)@ge;
+    }
+
+# Add space after (if, while, for, switch) and open parenthesis
+    $do_cvt = "space_after_if_while_for_switch";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\b(if|while|for|switch) \(@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\b(if|while|for|switch)[ \t]*\(@$1 \(@g;
+    }
+
+# Add space after comma
+    $do_cvt = "space_after_comma";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@,(?=[\w\(])@, @g;
+    }
+
+# Add spaces around logical tests
+    $do_cvt = "space_around_logical_tests";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@([\)\w]+)(==|!=|>|>=|<|<=|&&|\|\|)([\(\w\*\-])@$1 $2 $3@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Add spaces around assign
+    $do_cvt = "space_around_assign";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@([\)\w]+)(=|\+=|\-=|\*=|/=|>>=|<<=)([\(\w\*\-])@$1 $2 $3@g;
+    }
+
+# Add spaces around arithmetic
+    $do_cvt = "space_around_arithmetic";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@([\)\w]+)(\+|\-)([\(\w\*])@$1 $2 $3@g;
+    }
+
+# Add spaces around trigraph
+    $do_cvt = "space_around_trigraph";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@([\)\w\"]+) \? ([\(\)\[\]\w\*\" \t\.\>\-]*[^ \t]) \: ([\w\(\"\-])@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@([\)\w\"]+)[ \t]*\?[ \t]*([\(\)\[\]\w\*\" \t\.\>\-]*[^ \t])[ \t]*\:[ \t]*([\w\(\"\-])@$1 ? $2 : $3@g;
+    }
+
+# Use a space before a pointer,
+    $do_cvt = "space_before_pointer";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\bstruct \w+ \*@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+)([\t]+)\*[ \t]*@struct $1$2\*@g;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+) *\*[ \t]*@struct $1 \*@g;
+	$hash{$do_cvt} += $text =~ s@\bstruct\b\s+(\w+)([ \t]+)\*__@struct $1$2\* __@g;
+    }
+
+# Convert "for (foo;bar;baz)" to "for (foo; bar; baz)"
+    $do_cvt = "space_after_for_semicolons";
+    if (convert($do_cvt)) {
+	my $count;
+	($count, $text) = subst_line_mode_fn($text, '^([ \t]*)for\s*\([ \t]*([^;]+);[ \t]*([^;]+);[ \t]*([^\)]+)\)', 'check_for', '$1, $2, $3, $4');
+	$hash{$do_cvt} += $count;
+    }
+
+# cuddle open brace
+    $do_cvt = "cuddle_open_brace";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@(\)|\belse\b) \{\n@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@(\)|\belse\b|\bcase\s+\w+\s*:|\b(?:struct|union)[ \t]*(?:\w+|))[ \t]*[ \t]*\n[ \t]+\{[ \t]*\n@$1 \{\n@g;
+    }
+
+# cuddle else
+    $do_cvt = "cuddle_else";
+    if (convert($do_cvt)) {
+	my @matches = $text =~ m@\} else\b@g;
+	$hash{$do_cvt} -= @matches;
+	$hash{$do_cvt} += $text =~ s@\}[ \t]*\n[ \t]+else\b@\} else@g;
+    }
+
+# Remove multiple semicolons at end-of-line
+    $do_cvt = "coalesce_semicolons";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s@;[ \t]*;[ \t]*\n@;\n@g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Remove spaces before open bracket
+    $do_cvt = "remove_whitespace_before_bracket";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]+\[@\[@g;
+    }
+
+# Remove spaces after open parenthesis and before close parenthesis
+    $do_cvt = "remove_parenthesis_whitespace";
+    if (convert($do_cvt)) {
+	$text =~ s@[ \t]*\)@\)@g;
+	$text =~ s@\([ \t]*@\(@g;
+    }
+
+# Convert leading spaces to tabs
+    $do_cvt = "leading_spaces_to_tabs";
+    if (convert($do_cvt)) {
+	my $count;
+	($count, $text) = subst_line_mode_fn($text, '(^[ \t]+)', 'tabify', '$1');
+	$hash{$do_cvt} += $count;
+    }
+
+# Remove trailing whitespace
+    $do_cvt = "remove_trailing_whitespace";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]+\n@\n@g;
+	$hash{$do_cvt} += $text =~ s@\n+$@\n@g;
+    }
+
+# Remove whitespace before quoted newlines
+    $do_cvt = "remove_whitespace_before_quoted_newline";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(\"[^\"\n]*[^ \t])[ \t]+\\n@$1\\n@g;
+    }
+
+# Remove whitespace before trailing semicolon
+    $do_cvt = "remove_whitespace_before_trailing_semicolon";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(\n[^\n]+)\s+;[ \t]*\n$@$1;\n@g;
+    }
+
+# Remove whitespace after cast
+    $do_cvt = "remove_whitespace_after_cast";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]*\*[ \t]*\)[ \t]+@ \*\)@g;
+    }
+
+# Convert c99 comments to /* */ (don't convert (http|ftp)://)
+    $do_cvt = "convert_c99_comments";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@(?<!:)\/\/[ \t]*(.*)[ \t]*\n+@\/* $1 *\/\n@g;
+    }
+
+# Remove braces from single statements (not multiple-line single statements)
+    $do_cvt = "remove_single_statement_braces";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@[ \t]*\{[ \t]*\n([^;\{\n]+;)[ \t]*\n[ \t]+\}[ \t]*\n@\n$1\n@g;
+    }
+
+# Hoist assigns from if
+    $do_cvt = "hoist_assigns_from_if";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@\n([ \t]*)if\s*\(\s*([\!]{0,1})\s*\(\s*([\*\w\-\>\.\[\]]+)\s*=\s*(?=[^=])\s*([\w\-\>\.\* \t\[\]]*\s*${match_balanced_parentheses}*\s*(\?\:\&|\||\>\>|\<\<|\-|\+|\*|\/ \t)*\s*[\w\-\>\.\* \t\[\]]*\s*${match_balanced_parentheses}*)\s*\)@\n$1$3 = $4;\n$1if \($2$3@gx;
+    }
+
+# Remove casts of private_data
+    $do_cvt = "remove_private_data_casts";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s/=\s*\(\s*\w+\s*\w+\s*\*\s*\)\s*(\w+)->\s*private_data\b/= $1->private_data/g;
+    }
+
+# Remove static initializations to 0 or NULL
+
+    $do_cvt = "remove_static_initializations_to_0";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s/\b([ \t]*)static\s*([\w\t \*]+)\s*=\s*(0|NULL)\s*;/"$1static " . scalar(trim_trail($2)) . ";"/egx;
+    }
+
+# Convert "CamelCase" to "camel_case"
+    $do_cvt = "CamelCase_to_camel_case";
+    if (convert($do_cvt)) {
+	my $count = 0;
+	do {
+	    $count = $text =~ s/\b([A-Za-z])([a-z_]+)([A-Z])([a-zA-Z]+)\b/\L$1\E$2_\L$3\E$4/g;
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+    }
+
+# Remove comparisons to true or false
+    $do_cvt = "remove_true_false_comparisons";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s/(\([^\=\;\{\}\(]+)==\s*true\s*\)/"(" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+	$hash{$do_cvt} += $text =~ s/(\([^\!\;\{\}\(]+)!=\s*true\s*\)/"(!" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+	$hash{$do_cvt} += $text =~ s/(\([^\=\;\{\}\(]+)==\s*false\s*\)/"(!" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+	$hash{$do_cvt} += $text =~ s/(\([^\!\;\{\}\(]+)!=\s*false\s*\)/"(" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+    }
+
+# Remove comparisons to NULL
+    $do_cvt = "remove_NULL_comparisons";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s/(\([^\=\;\{\}\(]+)==\s*NULL\s*\)/"(!" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+	$hash{$do_cvt} += $text =~ s/(\([^\!\;\{\}\(]+)!=\s*NULL\s*\)/"(" . scalar(strip_leading_paren(trim_trail($1))) . ")"/egx;
+    }
+
+# Remove trailing if semicolons
+    $do_cvt = "remove_trailing_if_semicolons";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s/\bif(\s*)($match_balanced_parentheses)\s*;(?!\s*(else|\/\*))/if$1$2/g;
+    }
+
+# Convert normal comments to network comments
+    $do_cvt = "network_comments";
+    if (convert($do_cvt)) {
+	$hash{$do_cvt} += $text =~ s@/\*[ \t]*\n[ \t]*\*@/*@g;
+	$hash{$do_cvt} += $text =~ s@/\*([ \t]*)([^\n]+)\n[ \t]*\*/@/\*$1$2 \*/@g;
+    }
+
+# Remove unnecessary semicolons after switch () {};
+    $do_cvt = "remove_switchforwhileif_semicolons";
+    if (convert($do_cvt)) {
+	my $count = 0;
+        do {
+	    $count = 0;
+	    $count += $text =~ s@\b((?:switch|for|while|if)\s*${match_balanced_parentheses}\s*)${match_balanced_braces}\s*;@"$1$3"@egx;
+	    $hash{$do_cvt} += $count;
+        } while ($count > 0);
+    }
+
+# detab_else_return
+    $do_cvt = "detab_else_return";
+    if (convert($do_cvt)) {
+	my $count = 0;
+        do {
+	    $count = 0;
+	    $count += $text =~ s@(?!else\s+)\b(if\s*${match_balanced_parentheses}\s*)${match_balanced_braces}\s*else\s*\{?\s*return\s+([^;]+;)\s*\}?@"$1$3"@egx;
+	    $hash{$do_cvt} += $count;
+        } while ($count > 0);
+    }
+
+# Remove while while loops
+    $do_cvt = "remove_while_while";
+    if (convert($do_cvt)) {
+	my $count = 0;
+        do {
+	    $count = 0;
+	    $count += $text =~ s@(while\s*(${match_balanced_parentheses})\s*${match_balanced_braces})\s*while\s*\2\s*;@$1@egx;
+	    $hash{$do_cvt} += $count;
+        } while ($count > 0);
+    }
+
+# Remove fallthrough comments and convert to fallthrough;
+    $do_cvt = "fallthrough";
+    if (convert($do_cvt)) {
+
+	# 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++;
+		}
+	    }
+	    $hash{$do_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++;
+		}
+	    }
+	    $hash{$do_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++;
+		}
+	    }
+	    $hash{$do_cvt} += $count;
+	} while ($count > 0);
+
+    }
+
+# write the file if something was changed
+
+    if ($text ne $oldtext) {
+	my $newfile = $file;
+
+	$modified = 1;
+
+	if (!$overwrite) {
+	    $newfile = "$newfile$suffix";
+	}
+	open($f, '>', $newfile)
+	    or die "$P: Can't open $newfile for write\n";
+	print $f $text;
+	close($f);
+
+	if (!$quiet || $stats) {
+	    if ($overwrite) {
+		print "Converted $file\n";
+	    } else {
+		print "Converted $file to $newfile\n";
+	    }
+	}
+
+	if ($stats) {
+	    while ((my $key, my $value) = each(%hash)) {
+		next if ($value <= 0);
+		print "$value\t$key\n" if $value;
+		$hash{$key} = 0;	#Reset for next file
+	    }
+	}
+    }
+}
+
+
+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 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Miguel Ojeda]

Hi Joe,

On Sat, Oct 5, 2019 at 6:47 PM Joe Perches <joe@perches.com> wrote:
>
> diff --git a/scripts/cvt_style.pl b/scripts/cvt_style.pl
> new file mode 100755
> index 000000000000..fcbda0b1c67a
> --- /dev/null
> +++ b/scripts/cvt_style.pl
> @@ -0,0 +1,808 @@
> +#!/usr/bin/perl -w

Nit: #!/usr/bin/env perl instead?

> +
> +# Change some style elements of a source file
> +# An imperfect source code formatter.
> +# Might make trivial patches a bit easier.
> +#
> +# usage: perl scripts/cvt_kernel_style.pl <files>
> +#
> +# Licensed under the terms of the GNU GPL License version 2

Nit: use # SPDX-License-Identifier: GPL-2.0-only instead

As for the commit itself: while I am sure this tool is very useful
(and certainly you put a *lot* of effort into this tool), I don't see
how it is related to the fallthrough remapping (at least the
non-fallthrough parts).

Also, we should consider whether we want more tools like this now or
simply put the efforts into moving to clang-format.

Cheers,
Miguel

Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Joe Perches]

On Sat, 2019-10-05 at 19:31 +0200, Miguel Ojeda wrote:
> Hi Joe,

Hello.

> On Sat, Oct 5, 2019 at 6:47 PM Joe Perches <joe@perches.com> wrote:
[]
> > As for the commit itself: while I am sure this tool is very useful
> (and certainly you put a *lot* of effort into this tool), I don't see
> how it is related to the fallthrough remapping (at least the
> non-fallthrough parts).

It's not particularly related.

It's a 10 year old script that I just extended because it's
convenient for me.

I think I first posted it in 2011, but I started it as a
complement to checkpatch in 2010.

https://lwn.net/Articles/380161/

Doing the regexes for the fallthrough conversions took me
a couple hours.

> Also, we should consider whether we want more tools like this now or
> simply put the efforts into moving to clang-format.

I think clang-format could not do this sort of conversion.
Nor could coccinelle or checkpatch.

Anyway, it's not really necessary for this particular patch
to be applied, but it's a convenient way to show the script
has the capability to do fallthrough comment conversions.

I think it does conversions fairly reasonably but likely
some files could not compile without adding an #include
like:

#include <linux/compiler.h>

Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Kees Cook]

On Sat, Oct 05, 2019 at 11:35:42PM -0700, Joe Perches wrote:
> On Sat, 2019-10-05 at 19:31 +0200, Miguel Ojeda wrote:
> > Hi Joe,
> 
> Hello.
> 
> > On Sat, Oct 5, 2019 at 6:47 PM Joe Perches <joe@perches.com> wrote:
> []
> > > As for the commit itself: while I am sure this tool is very useful
> > (and certainly you put a *lot* of effort into this tool), I don't see
> > how it is related to the fallthrough remapping (at least the
> > non-fallthrough parts).
> 
> It's not particularly related.
> 
> It's a 10 year old script that I just extended because it's
> convenient for me.
> 
> I think I first posted it in 2011, but I started it as a
> complement to checkpatch in 2010.
> 
> https://lwn.net/Articles/380161/
> 
> Doing the regexes for the fallthrough conversions took me
> a couple hours.
> 
> > Also, we should consider whether we want more tools like this now or
> > simply put the efforts into moving to clang-format.
> 
> I think clang-format could not do this sort of conversion.
> Nor could coccinelle or checkpatch.
> 
> Anyway, it's not really necessary for this particular patch
> to be applied, but it's a convenient way to show the script
> has the capability to do fallthrough comment conversions.
> 
> I think it does conversions fairly reasonably but likely
> some files could not compile without adding an #include
> like:
> 
> #include <linux/compiler.h>

I think this is a nice tool to add -- at the very least it serves as
infrastructure for future similar conversions. And small cleanups can be
generated from it for people looking to clean up subsystems, etc.

-- 
Kees Cook

Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Joe Perches]

On Thu, 2019-10-10 at 13:39 -0700, Kees Cook wrote:
> On Sat, Oct 05, 2019 at 11:35:42PM -0700, Joe Perches wrote:
[]
> > I think clang-format could not do this sort of conversion.
> > Nor could coccinelle or checkpatch.
> > 
> > Anyway, it's not really necessary for this particular patch
> > to be applied, but it's a convenient way to show the script
> > has the capability to do fallthrough comment conversions.
> > 
> > I think it does conversions fairly reasonably but likely
> > some files could not compile without adding an #include
> > like:
> > 
> > #include <linux/compiler.h>
> 
> I think this is a nice tool to add -- at the very least it serves as
> infrastructure for future similar conversions. And small cleanups can be
> generated from it for people looking to clean up subsystems, etc.

Another similar tool that used checkpatch and also compile
tested and created git commits was reformat_with_checkpatch

https://lore.kernel.org/lkml/1405128087.6751.12.camel@joe-AO725/

Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways

[Nick Desaulniers]

On Sat, Oct 5, 2019 at 9:47 AM Joe Perches <joe@perches.com> wrote:
>
> Trivial tool to reformat source code in various ways.
>
> This is an old tool that was recently updated to convert /* fallthrough */
> style comments to the new pseudo-keyword fallthrough;
>
> Typical command line use is:
>     $ perl scripts/cvt_style --convert=fallthrough <file list>

It would be cool to include the treewide onliner from your cover sheet
in this commit message, as I find myself flipping between that and
this, otherwise the recommended onliner will be lost to LKML (instead
of being lost to git log). Or in the usage comment at the top of the
script.

>
> Available conversions:
>         all
>         printk_to_pr_level
>         printk_KERN_DEBUG_to_pr_debug
>         dev_printk_to_dev_level
>         dev_printk_KERN_DEBUG_to_dev_dbg
>         sdev_printk_to_sdev_level
>         sdev_printk_KERN_DEBUG_to_sdev_dbg
>         coalesce_formats
>         cuddle_open_brace
>         cuddle_else

I think some of these could use examples of what they do. I can't read
perl (as we've previously established :P) and I'm not sure what it
means to cuddle open braces or elses, though they do sound nice.

>         deparenthesize_returns
>         space_after_KERN_level
>         space_after_if_while_for_switch
>         space_after_for_semicolons
>         space_after_comma
>         space_before_pointer
>         space_around_trigraph
>         leading_spaces_to_tabs
>         coalesce_semicolons
>         remove_trailing_whitespace
>         remove_whitespace_before_quoted_newline
>         remove_whitespace_before_trailing_semicolon
>         remove_whitespace_before_bracket
>         remove_parenthesis_whitespace
>         remove_single_statement_braces
>         remove_whitespace_after_cast
>         hoist_assigns_from_if
>         convert_c99_comments
>         remove_private_data_casts
>         remove_static_initializations_to_0
>         remove_true_false_comparisons
>         remove_NULL_comparisons
>         remove_trailing_if_semicolons

To Miguel's comment about clang-format, it looks like you can do:

Use -style="{key: value, ...}" to set specific
                              parameters, e.g.:
                                -style="{BasedOnStyle: llvm, IndentWidth: 8}"

via: https://clang.llvm.org/docs/ClangFormat.html
which might make some nice one liners for some of these.


>         network_comments
>         remove_switchforwhileif_semicolons
>         detab_else_return
>         remove_while_while
>         fallthrough
> Additional conversions which may not work well:
>         (enable individually or with --convert=all --broken)
>         move_labels_to_column_1
>         space_around_logical_tests
>         space_around_assign
>         space_around_arithmetic
>         CamelCase_to_camel_case

s/camel_case/snake_case/

I'll give the script a run later this week and report back if I can
find any errors in the resulting build, as in the previous patch
series. Thanks for the work on this.
-- 
Thanks,
~Nick Desaulniers

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