| Message ID | 7aa0ec0b-c00e-44c0-1c07-6bd87e2b6c47@users.sourceforge.net (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Herbert Xu |
| Headers | show |
Am Samstag, 21. Oktober 2017, 19:53:54 CEST schrieb SF Markus Elfring: Hi Markus, > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 21 Oct 2017 19:29:11 +0200 > > Adjust jump targets so that a bit of exception handling can be better > reused at the end of this function. > > This issue was detected by using the Coccinelle software. Thank you for this patch. > -outbuf: > + ret = memcmp(test->expected, buf, test->expectedlen); > +free_rng: > crypto_free_rng(drng); > +free_buffer: > kzfree(buf); > return ret; > + > +report_failure: > + pr_err("alg: drbg: could not obtain random data for driver %s\n", > + driver); > + goto free_rng; Though, jumping back and forth like this with goto directives is something that looks a bit strange. At least to my taste, may I suggest to have gotos pointing only downwards and not up again? (Note, the same applies to the ansi_cprng patch set). What about something like following: ... memcmp goto free_rng; report_failure: <failure report> free_rng: <the deallocation code> Ciao Stephan
> Though, jumping back and forth like this with goto directives is something > that looks a bit strange. At least to my taste, may I suggest to have gotos > pointing only downwards and not up again? (Note, the same applies to the > ansi_cprng patch set). > > What about something like following: > > ... > memcmp > goto free_rng; Do you find an additional jump really acceptable at such a source code place? > report_failure: > <failure report> > > free_rng: > <the deallocation code> I am curious on how feedback will evolve also for the other design approach. Regards, Markus
diff --git a/crypto/testmgr.c b/crypto/testmgr.c index dd9c7f1c1c7b..76c5128284f8 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -1914,8 +1914,8 @@ static int drbg_cavs_test(const struct drbg_testvec *test, int pr, if (IS_ERR(drng)) { printk(KERN_ERR "alg: drbg: could not allocate DRNG handle for " "%s\n", driver); - kzfree(buf); - return -ENOMEM; + ret = -ENOMEM; + goto free_buffer; } test_data.testentropy = &testentropy; @@ -1924,7 +1924,7 @@ static int drbg_cavs_test(const struct drbg_testvec *test, int pr, ret = crypto_drbg_reset_test(drng, &pers, &test_data); if (ret) { printk(KERN_ERR "alg: drbg: Failed to reset rng\n"); - goto outbuf; + goto free_rng; } drbg_string_fill(&addtl, test->addtla, test->addtllen); @@ -1936,11 +1936,9 @@ static int drbg_cavs_test(const struct drbg_testvec *test, int pr, ret = crypto_drbg_get_bytes_addtl(drng, buf, test->expectedlen, &addtl); } - if (ret < 0) { - printk(KERN_ERR "alg: drbg: could not obtain random data for " - "driver %s\n", driver); - goto outbuf; - } + + if (ret < 0) + goto report_failure; drbg_string_fill(&addtl, test->addtlb, test->addtllen); if (pr) { @@ -1951,18 +1949,21 @@ static int drbg_cavs_test(const struct drbg_testvec *test, int pr, ret = crypto_drbg_get_bytes_addtl(drng, buf, test->expectedlen, &addtl); } - if (ret < 0) { - printk(KERN_ERR "alg: drbg: could not obtain random data for " - "driver %s\n", driver); - goto outbuf; - } - ret = memcmp(test->expected, buf, test->expectedlen); + if (ret < 0) + goto report_failure; -outbuf: + ret = memcmp(test->expected, buf, test->expectedlen); +free_rng: crypto_free_rng(drng); +free_buffer: kzfree(buf); return ret; + +report_failure: + pr_err("alg: drbg: could not obtain random data for driver %s\n", + driver); + goto free_rng; }