From: Joe Perches <joe@perches.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Keith Busch <kbusch@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
linux-crypto@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
Jiri Kosina <trivial@kernel.org>,
Kees Cook <kees.cook@canonical.com>,
Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
Date: Wed, 09 Sep 2020 17:20:42 -0700 [thread overview]
Message-ID: <c50014e57e77842e4c710231dff829f3f67bbe59.camel@perches.com> (raw)
In-Reply-To: <20200909223540.GA27590@gondor.apana.org.au>
On Thu, 2020-09-10 at 08:35 +1000, Herbert Xu wrote:
> On Wed, Sep 09, 2020 at 02:09:32PM -0700, Joe Perches wrote:
> > On Wed, 2020-09-09 at 13:55 -0700, Keith Busch wrote:
> > > On Wed, Sep 09, 2020 at 01:06:39PM -0700, Joe Perches wrote:
> > > > diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> > > > index eea0f453cfb6..8aac5bc60f4c 100644
> > > > --- a/crypto/tcrypt.c
> > > > +++ b/crypto/tcrypt.c
> > > > @@ -2464,7 +2464,7 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
> > > > test_hash_speed("streebog512", sec,
> > > > generic_hash_speed_template);
> > > > if (mode > 300 && mode < 400) break;
> > > > - fallthrough;
> > > > + break;
> > > > case 399:
> > > > break;
> > >
> > > Just imho, this change makes the preceding 'if' look even more
> > > pointless. Maybe the fallthrough was a deliberate choice? Not that my
> > > opinion matters here as I don't know this module, but it looked a bit
> > > odd to me.
> >
> > It does look odd to me too.
> >
> > It's also just a test module though so the
> > code isn't particularly crucial.
> >
> > Herbert/David? thoughts?
>
> Please read the function as a whole, that fallthrough (and every other
> one in do_test) needs to stay.
Hi Herbert.
OK, I did. I think it's odd code.
The do_test function could start at any 3xx or 4xx mode
and then do all the next tests to the highest 99
for ranges from 301-399 and 401-499.
ie: Pass mode 312, it'll execute test 312 and every
other test to 399.
Not the same for the 500-599 or 600-699 range.
I probably would've hoisted the code for all of the 0,
any value from 300-399, and any value from 400-499
out of the do_test function and into tcrypt_mod_init
not used fallthrough in do_test at all.
cheers, Joe
Perhaps something like:
---
crypto/tcrypt.c | 190 +++++++++++++++++++++-----------------------------------
1 file changed, 72 insertions(+), 118 deletions(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index eea0f453cfb6..aeb089f6ee27 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1660,7 +1660,6 @@ static inline int tcrypt_test(const char *alg)
static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
{
- int i;
int ret = 0;
switch (m) {
@@ -1672,8 +1671,6 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
break;
}
- for (i = 1; i < 200; i++)
- ret += do_test(NULL, 0, 0, i, num_mb);
break;
case 1:
@@ -2349,123 +2346,93 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
test_hash_speed(alg, sec, generic_hash_speed_template);
break;
}
- fallthrough;
+ break;
case 301:
test_hash_speed("md4", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 302:
test_hash_speed("md5", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 303:
test_hash_speed("sha1", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 304:
test_hash_speed("sha256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 305:
test_hash_speed("sha384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 306:
test_hash_speed("sha512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 307:
test_hash_speed("wp256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 308:
test_hash_speed("wp384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 309:
test_hash_speed("wp512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 310:
test_hash_speed("tgr128", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 311:
test_hash_speed("tgr160", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 312:
test_hash_speed("tgr192", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 313:
test_hash_speed("sha224", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 314:
test_hash_speed("rmd128", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 315:
test_hash_speed("rmd160", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 316:
test_hash_speed("rmd256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 317:
test_hash_speed("rmd320", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 318:
klen = 16;
test_hash_speed("ghash", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 319:
test_hash_speed("crc32c", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 320:
test_hash_speed("crct10dif", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 321:
test_hash_speed("poly1305", sec, poly1305_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 322:
test_hash_speed("sha3-224", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 323:
test_hash_speed("sha3-256", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 324:
test_hash_speed("sha3-384", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 325:
test_hash_speed("sha3-512", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 326:
test_hash_speed("sm3", sec, generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 327:
test_hash_speed("streebog256", sec,
generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
+ break;
case 328:
test_hash_speed("streebog512", sec,
generic_hash_speed_template);
- if (mode > 300 && mode < 400) break;
- fallthrough;
- case 399:
break;
case 400:
@@ -2473,122 +2440,93 @@ static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
test_ahash_speed(alg, sec, generic_hash_speed_template);
break;
}
- fallthrough;
+ break;
case 401:
test_ahash_speed("md4", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 402:
test_ahash_speed("md5", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 403:
test_ahash_speed("sha1", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 404:
test_ahash_speed("sha256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 405:
test_ahash_speed("sha384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 406:
test_ahash_speed("sha512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 407:
test_ahash_speed("wp256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 408:
test_ahash_speed("wp384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 409:
test_ahash_speed("wp512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 410:
test_ahash_speed("tgr128", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 411:
test_ahash_speed("tgr160", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 412:
test_ahash_speed("tgr192", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 413:
test_ahash_speed("sha224", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 414:
test_ahash_speed("rmd128", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 415:
test_ahash_speed("rmd160", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 416:
test_ahash_speed("rmd256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 417:
test_ahash_speed("rmd320", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 418:
test_ahash_speed("sha3-224", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 419:
test_ahash_speed("sha3-256", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 420:
test_ahash_speed("sha3-384", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 421:
test_ahash_speed("sha3-512", sec, generic_hash_speed_template);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 422:
test_mb_ahash_speed("sha1", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 423:
test_mb_ahash_speed("sha256", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 424:
test_mb_ahash_speed("sha512", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 425:
test_mb_ahash_speed("sm3", sec, generic_hash_speed_template,
num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 426:
test_mb_ahash_speed("streebog256", sec,
generic_hash_speed_template, num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
+ break;
case 427:
test_mb_ahash_speed("streebog512", sec,
generic_hash_speed_template, num_mb);
- if (mode > 400 && mode < 500) break;
- fallthrough;
- case 499:
break;
case 500:
@@ -3034,7 +2972,23 @@ static int __init tcrypt_mod_init(void)
goto err_free_tv;
}
- err = do_test(alg, type, mask, mode, num_mb);
+ if (mode == 0) {
+ err = do_test(alg, type, mask, mode, num_mb);
+ if (!err) {
+ for (i = 1; i < 200; i++)
+ err |= do_test(NULL, 0, 0, i, num_mb);
+ }
+ } else if (mode >= 300 && mode < 400) {
+ err = 0;
+ for (i = mode; i < 400; i++)
+ err |= do_test(alg, type, mask, i, num_mb);
+ } else if (mode >= 400 && mode < 500) {
+ err = 0;
+ for (i = mode; i < 500; i++)
+ err |= do_test(alg, type, mask, i, num_mb);
+ } else {
+ err = do_test(alg, type, mask, mode, num_mb);
+ }
if (err) {
printk(KERN_ERR "tcrypt: one or more tests failed!\n");
next prev parent reply other threads:[~2020-09-10 0:20 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-09 20:06 [trivial PATCH] treewide: Convert switch/case fallthrough; to break; Joe Perches
2020-09-09 20:38 ` Gustavo A. R. Silva
2020-09-09 20:55 ` Keith Busch
2020-09-09 21:09 ` Joe Perches
2020-09-09 22:35 ` Herbert Xu
2020-09-10 0:20 ` Joe Perches [this message]
2020-09-17 19:40 ` Jacob Keller
2020-09-09 22:36 ` Jason Gunthorpe
2020-09-09 22:47 ` Joe Perches
2020-09-10 6:28 ` Mauro Carvalho Chehab
2020-09-10 6:53 ` Wolfram Sang
2020-09-10 8:24 ` Nicolas.Ferre
2020-09-10 8:47 ` Felipe Balbi
2020-09-10 9:18 ` Steffen Maier
2020-09-10 9:24 ` Ilya Dryomov
2020-09-10 10:16 ` Matthias Brugger
2020-09-10 14:21 ` Robin Murphy
2020-09-11 4:19 ` Joe Perches
2020-09-15 9:06 ` [Intel-gfx] " Jani Nikula
2020-09-15 9:19 ` Miquel Raynal
2020-09-17 8:03 ` [oss-drivers] " Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c50014e57e77842e4c710231dff829f3f67bbe59.camel@perches.com \
--to=joe@perches.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=kbusch@kernel.org \
--cc=kees.cook@canonical.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ndesaulniers@google.com \
--cc=trivial@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox