* [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword @ 2019-10-05 16:46 Joe Perches 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches ` (4 more replies) 0 siblings, 5 replies; 27+ messages in thread From: Joe Perches @ 2019-10-05 16:46 UTC (permalink / raw) To: Linus Torvalds, linux-sctp Cc: Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, linux-doc, netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches @ 2019-10-05 16:46 ` Joe Perches 2019-10-07 18:08 ` Nick Desaulniers ` (2 more replies) 2019-10-05 16:46 ` [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use Joe Perches ` (3 subsequent siblings) 4 siblings, 3 replies; 27+ messages in thread From: Joe Perches @ 2019-10-05 16:46 UTC (permalink / raw) To: Linus Torvalds, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner Cc: Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, linux-sctp, netdev 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches @ 2019-10-07 18:08 ` Nick Desaulniers 2019-10-10 20:34 ` Kees Cook 2019-10-11 12:20 ` Neil Horman 2 siblings, 0 replies; 27+ messages in thread From: Nick Desaulniers @ 2019-10-07 18:08 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), LKML, Nathan Chancellor, Andrew Morton, David Miller, clang-built-linux, linux-sctp, Network Development 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches 2019-10-07 18:08 ` Nick Desaulniers @ 2019-10-10 20:34 ` Kees Cook 2019-10-11 12:20 ` Neil Horman 2 siblings, 0 replies; 27+ messages in thread From: Kees Cook @ 2019-10-10 20:34 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, Miguel Ojeda, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, linux-sctp, netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches 2019-10-07 18:08 ` Nick Desaulniers 2019-10-10 20:34 ` Kees Cook @ 2019-10-11 12:20 ` Neil Horman 2 siblings, 0 replies; 27+ messages in thread From: Neil Horman @ 2019-10-11 12:20 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, Vlad Yasevich, Marcelo Ricardo Leitner, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, linux-sctp, netdev 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> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches @ 2019-10-05 16:46 ` Joe Perches 2019-10-05 17:17 ` Miguel Ojeda 2019-10-05 16:46 ` [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword Joe Perches ` (2 subsequent siblings) 4 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2019-10-05 16:46 UTC (permalink / raw) To: Linus Torvalds, Miguel Ojeda Cc: Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-05 16:46 ` [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use Joe Perches @ 2019-10-05 17:17 ` Miguel Ojeda 2019-10-07 18:14 ` Nick Desaulniers 2019-10-10 20:37 ` Kees Cook 0 siblings, 2 replies; 27+ messages in thread From: Miguel Ojeda @ 2019-10-05 17:17 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-05 17:17 ` Miguel Ojeda @ 2019-10-07 18:14 ` Nick Desaulniers 2019-10-07 18:28 ` Joe Perches 2019-10-10 20:37 ` Kees Cook 1 sibling, 1 reply; 27+ messages in thread From: Nick Desaulniers @ 2019-10-07 18:14 UTC (permalink / raw) To: Miguel Ojeda Cc: Joe Perches, Linus Torvalds, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-kernel, Nathan Chancellor, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-07 18:14 ` Nick Desaulniers @ 2019-10-07 18:28 ` Joe Perches 0 siblings, 0 replies; 27+ messages in thread From: Joe Perches @ 2019-10-07 18:28 UTC (permalink / raw) To: Nick Desaulniers, Miguel Ojeda Cc: Linus Torvalds, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-kernel, Nathan Chancellor, Andrew Morton, David Miller, clang-built-linux 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/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-05 17:17 ` Miguel Ojeda 2019-10-07 18:14 ` Nick Desaulniers @ 2019-10-10 20:37 ` Kees Cook 2019-10-11 22:25 ` Miguel Ojeda 1 sibling, 1 reply; 27+ messages in thread From: Kees Cook @ 2019-10-10 20:37 UTC (permalink / raw) To: Miguel Ojeda Cc: Joe Perches, Linus Torvalds, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use 2019-10-10 20:37 ` Kees Cook @ 2019-10-11 22:25 ` Miguel Ojeda 0 siblings, 0 replies; 27+ messages in thread From: Miguel Ojeda @ 2019-10-11 22:25 UTC (permalink / raw) To: Kees Cook Cc: Joe Perches, Linus Torvalds, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-kernel, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches 2019-10-05 16:46 ` [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use Joe Perches @ 2019-10-05 16:46 ` Joe Perches 2019-10-05 17:47 ` Miguel Ojeda 2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches 2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds 4 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2019-10-05 16:46 UTC (permalink / raw) To: Linus Torvalds, linux-kernel Cc: Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, linux-doc 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword 2019-10-05 16:46 ` [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword Joe Perches @ 2019-10-05 17:47 ` Miguel Ojeda 2019-10-09 21:51 ` Nick Desaulniers 0 siblings, 1 reply; 27+ messages in thread From: Miguel Ojeda @ 2019-10-05 17:47 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, linux-kernel, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Linux Doc Mailing List 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword 2019-10-05 17:47 ` Miguel Ojeda @ 2019-10-09 21:51 ` Nick Desaulniers 0 siblings, 0 replies; 27+ messages in thread From: Nick Desaulniers @ 2019-10-09 21:51 UTC (permalink / raw) To: Miguel Ojeda Cc: Joe Perches, Linus Torvalds, linux-kernel, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Linux Doc Mailing List 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches ` (2 preceding siblings ...) 2019-10-05 16:46 ` [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword Joe Perches @ 2019-10-05 16:46 ` Joe Perches 2019-10-05 17:31 ` Miguel Ojeda 2019-10-09 22:37 ` Nick Desaulniers 2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds 4 siblings, 2 replies; 27+ messages in thread From: Joe Perches @ 2019-10-05 16:46 UTC (permalink / raw) To: Linus Torvalds, linux-kernel Cc: Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, x86, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches @ 2019-10-05 17:31 ` Miguel Ojeda 2019-10-06 6:35 ` Joe Perches 2019-10-09 22:37 ` Nick Desaulniers 1 sibling, 1 reply; 27+ messages in thread From: Miguel Ojeda @ 2019-10-05 17:31 UTC (permalink / raw) To: Joe Perches Cc: Linus Torvalds, linux-kernel, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-05 17:31 ` Miguel Ojeda @ 2019-10-06 6:35 ` Joe Perches 2019-10-10 20:39 ` Kees Cook 0 siblings, 1 reply; 27+ messages in thread From: Joe Perches @ 2019-10-06 6:35 UTC (permalink / raw) To: Miguel Ojeda Cc: Linus Torvalds, linux-kernel, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-06 6:35 ` Joe Perches @ 2019-10-10 20:39 ` Kees Cook 2019-10-10 20:48 ` Joe Perches 0 siblings, 1 reply; 27+ messages in thread From: Kees Cook @ 2019-10-10 20:39 UTC (permalink / raw) To: Joe Perches Cc: Miguel Ojeda, Linus Torvalds, linux-kernel, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-10 20:39 ` Kees Cook @ 2019-10-10 20:48 ` Joe Perches 0 siblings, 0 replies; 27+ messages in thread From: Joe Perches @ 2019-10-10 20:48 UTC (permalink / raw) To: Kees Cook Cc: Miguel Ojeda, Linus Torvalds, linux-kernel, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux 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/ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways 2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches 2019-10-05 17:31 ` Miguel Ojeda @ 2019-10-09 22:37 ` Nick Desaulniers 1 sibling, 0 replies; 27+ messages in thread From: Nick Desaulniers @ 2019-10-09 22:37 UTC (permalink / raw) To: Joe Perches, Miguel Ojeda Cc: Linus Torvalds, LKML, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Nathan Chancellor, Andrew Morton, David Miller, clang-built-linux 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches ` (3 preceding siblings ...) 2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches @ 2019-10-11 16:29 ` Linus Torvalds 2019-10-11 17:43 ` Joe Perches 2019-10-11 18:01 ` Miguel Ojeda 4 siblings, 2 replies; 27+ messages in thread From: Linus Torvalds @ 2019-10-11 16:29 UTC (permalink / raw) To: Joe Perches Cc: linux-sctp, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds @ 2019-10-11 17:43 ` Joe Perches 2019-10-11 17:46 ` Linus Torvalds 2019-10-11 18:01 ` Miguel Ojeda 1 sibling, 1 reply; 27+ messages in thread From: Joe Perches @ 2019-10-11 17:43 UTC (permalink / raw) To: Linus Torvalds Cc: linux-sctp, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 17:43 ` Joe Perches @ 2019-10-11 17:46 ` Linus Torvalds 2019-10-12 2:14 ` Joe Perches 0 siblings, 1 reply; 27+ messages in thread From: Linus Torvalds @ 2019-10-11 17:46 UTC (permalink / raw) To: Joe Perches Cc: linux-sctp, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 17:46 ` Linus Torvalds @ 2019-10-12 2:14 ` Joe Perches 0 siblings, 0 replies; 27+ messages in thread From: Joe Perches @ 2019-10-12 2:14 UTC (permalink / raw) To: Linus Torvalds Cc: linux-sctp, Miguel Ojeda, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds 2019-10-11 17:43 ` Joe Perches @ 2019-10-11 18:01 ` Miguel Ojeda 2019-10-11 22:07 ` Kees Cook 1 sibling, 1 reply; 27+ messages in thread From: Miguel Ojeda @ 2019-10-11 18:01 UTC (permalink / raw) To: Linus Torvalds Cc: Joe Perches, linux-sctp, Kees Cook, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 18:01 ` Miguel Ojeda @ 2019-10-11 22:07 ` Kees Cook 2019-10-11 22:26 ` Miguel Ojeda 0 siblings, 1 reply; 27+ messages in thread From: Kees Cook @ 2019-10-11 22:07 UTC (permalink / raw) To: Miguel Ojeda Cc: Linus Torvalds, Joe Perches, linux-sctp, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword 2019-10-11 22:07 ` Kees Cook @ 2019-10-11 22:26 ` Miguel Ojeda 0 siblings, 0 replies; 27+ messages in thread From: Miguel Ojeda @ 2019-10-11 22:26 UTC (permalink / raw) To: Kees Cook Cc: Linus Torvalds, Joe Perches, linux-sctp, Borislav Petkov, H . Peter Anvin, Thomas Gleixner, Pavel Machek, Gustavo A . R . Silva, Arnaldo Carvalho de Melo, Kan Liang, Namhyung Kim, Jiri Olsa, Alexander Shishkin, Shawn Landden, the arch/x86 maintainers, Linux Kernel Mailing List, Nathan Chancellor, Nick Desaulniers, Andrew Morton, David Miller, clang-built-linux, Jonathan Corbet, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, open list:DOCUMENTATION, Netdev 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-10-12 2:14 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-05 16:46 [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Joe Perches 2019-10-05 16:46 ` [PATCH 1/4] net: sctp: Rename fallthrough label to unhandled Joe Perches 2019-10-07 18:08 ` Nick Desaulniers 2019-10-10 20:34 ` Kees Cook 2019-10-11 12:20 ` Neil Horman 2019-10-05 16:46 ` [PATCH 2/4] compiler_attributes.h: Add 'fallthrough' pseudo keyword for switch/case use Joe Perches 2019-10-05 17:17 ` Miguel Ojeda 2019-10-07 18:14 ` Nick Desaulniers 2019-10-07 18:28 ` Joe Perches 2019-10-10 20:37 ` Kees Cook 2019-10-11 22:25 ` Miguel Ojeda 2019-10-05 16:46 ` [PATCH 3/4] Documentation/process: Add fallthrough pseudo-keyword Joe Perches 2019-10-05 17:47 ` Miguel Ojeda 2019-10-09 21:51 ` Nick Desaulniers 2019-10-05 16:46 ` [PATCH 4/4] scripts/cvt_style.pl: Tool to reformat sources in various ways Joe Perches 2019-10-05 17:31 ` Miguel Ojeda 2019-10-06 6:35 ` Joe Perches 2019-10-10 20:39 ` Kees Cook 2019-10-10 20:48 ` Joe Perches 2019-10-09 22:37 ` Nick Desaulniers 2019-10-11 16:29 ` [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword Linus Torvalds 2019-10-11 17:43 ` Joe Perches 2019-10-11 17:46 ` Linus Torvalds 2019-10-12 2:14 ` Joe Perches 2019-10-11 18:01 ` Miguel Ojeda 2019-10-11 22:07 ` Kees Cook 2019-10-11 22:26 ` Miguel Ojeda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).