linux-kernel - Re: [PATCH] cvt_fallthrough: A tool to convert /* fallthrough */ comments to fallthrough;

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Feb 2020 18:24:38 -0800
From:   Joe Perches <joe@...ches.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Subject: Re: [PATCH] cvt_fallthrough: A tool to convert /* fallthrough */
 comments to fallthrough;

On Thu, 2020-02-20 at 16:21 -0800, Andrew Morton wrote:
> On Thu, 20 Feb 2020 12:30:21 -0800 Joe Perches <joe@...ches.com> wrote:
> 
> > 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 overwrites the input files.
> > 
> > Typical command use:
> >     ./scripts/cvt_fallthrough.pl <path|file>
> > 
> > i.e.:
> > 
> >    $ ./scripts/cvt_fallthrough.pl block
> >      converts all files in block and its subdirectories
> >    $ ./scripts/cvt_fallthrough.pl drivers/net/wireless/zydas/zd1201.c
> >      converts a single file
> > 
> > A fallthrough comment with additional comments is converted akin to:
> > 
> > -		/* fall through - maybe userspace knows this conn_id. */
> > +		fallthrough;    /* maybe userspace knows this conn_id */
> > 
> > A fallthrough comment or fallthrough; between successive case statements
> > is deleted.
> > 
> > e.g.:
> > 
> >     case FOO:
> >     	/* fallthrough */ (or fallthrough;)
> >     case BAR:
> > 
> > is converted to:
> > 
> >     case FOO:
> >     case BAR:
> > 
> > Signed-off-by: Joe Perches <joe@...ches.com>
> > ---
> >  scripts/cvt_fallthrough.pl | 215 +++++++++++++++++++++++++++++++++++++
> 
> Do we need this in the tree long-term?

Out-of-tree scripts aren't used by trivial patch submitters.

So no, not really.  I think it's a 'good to have, short term'
script useful until at least most all of the conversions occur.

And I think having multiple concurrent styles for fallthrough
isn't great.

And I don't have the patience of someone like Gustavo Silva to
painstakin
gly shepherd hundreds of little patches either.
(thanks Gustavo, good
work...)

And it would be nice though to have some mechanism to get
scripted patches applied, either by subsystem or treewide.

> Or is it a matters of "hey
> Linus, please run this" then something like add a checkpatch rule to
> catch future slipups?

The checkpatch rule was added a week ago.
https://lore.kernel.org/lkml/8b6c1b9031ab9f3cdebada06b8d46467f1492d68.camel@perches.com/

Adding a --fix option wouldn't work as well as this script
to do conversions as the script is moderately complicated.

It does seem a treewide conversion like this could have a
small impact on stable trees, so any conversion should
probably be done by subsystem.

Anyway, the script does a pretty reasonable job at
conversions of the various styles of fallthrough comments.

Though there are some conversion that are not done when a
/* fallthrough */ comment is followed by another comment
before another case like:

	case FOO:
		/* fall through */
		/* another comment */
	case BAR:

Anyway, using today's -next, a treewide diffstat is:

$ git diff --shortstat
 1861 files changed, 4113 insertions(+), 4762 deletions(-)

And these are the most common conversions:

   2278 /* fall through */
    441 /* Fall through */
    253 /* fallthrough */
    164 /* FALLTHROUGH */
    116 /* fall-through */
     84 /* Fallthrough */
     33 /* FALL THROUGH */
     31 /* Fall through */				\
     27 /* FALLTHRU */
     24 /*FALLTHRU*/
     24 /* fallthru */
     19 /* fall thru */
     19 /* Fall Through */
     19 /* Fall-through */
     16 /* Else, fall through */
     15 /* fall-thru */
     13 /* Intentional fallthrough */
     13 /*FALLTHROUGH*/
     12 /* Fall thru */
     12 /* else, fall through */
     10 /*lint -fallthrough */
      9 /* Fall through. */
      9 }	/* fallthrough */
      8 /*fall through*/