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");


Reply via email to