* [PATCH 0/2] crypto-testmgr: Fine-tuning for drbg_cavs_test()
@ 2017-10-21 17:52 SF Markus Elfring
2017-10-21 17:53 ` [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test() SF Markus Elfring
2017-10-21 17:55 ` [PATCH 2/2] crypto-testmgr: Delete an unnecessary variable initialisation " SF Markus Elfring
0 siblings, 2 replies; 5+ messages in thread
From: SF Markus Elfring @ 2017-10-21 17:52 UTC (permalink / raw)
To: linux-crypto, David S. Miller, Herbert Xu; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Oct 2017 19:47:43 +0200
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Use common error handling code
Delete an unnecessary variable initialisation
crypto/testmgr.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
--
2.14.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test()
2017-10-21 17:52 [PATCH 0/2] crypto-testmgr: Fine-tuning for drbg_cavs_test() SF Markus Elfring
@ 2017-10-21 17:53 ` SF Markus Elfring
2017-10-21 19:07 ` Stephan Mueller
2017-10-21 17:55 ` [PATCH 2/2] crypto-testmgr: Delete an unnecessary variable initialisation " SF Markus Elfring
1 sibling, 1 reply; 5+ messages in thread
From: SF Markus Elfring @ 2017-10-21 17:53 UTC (permalink / raw)
To: linux-crypto, David S. Miller, Herbert Xu; +Cc: LKML, kernel-janitors
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.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
crypto/testmgr.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
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;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] crypto-testmgr: Delete an unnecessary variable initialisation in drbg_cavs_test()
2017-10-21 17:52 [PATCH 0/2] crypto-testmgr: Fine-tuning for drbg_cavs_test() SF Markus Elfring
2017-10-21 17:53 ` [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test() SF Markus Elfring
@ 2017-10-21 17:55 ` SF Markus Elfring
1 sibling, 0 replies; 5+ messages in thread
From: SF Markus Elfring @ 2017-10-21 17:55 UTC (permalink / raw)
To: linux-crypto, David S. Miller, Herbert Xu; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Oct 2017 19:33:45 +0200
The variable "ret" will eventually be set to an error code a bit later.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
crypto/testmgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 76c5128284f8..e46cf2c92497 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1901,7 +1901,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
static int drbg_cavs_test(const struct drbg_testvec *test, int pr,
const char *driver, u32 type, u32 mask)
{
- int ret = -EAGAIN;
+ int ret;
struct crypto_rng *drng;
struct drbg_test_data test_data;
struct drbg_string addtl, pers, testentropy;
--
2.14.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test()
2017-10-21 17:53 ` [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test() SF Markus Elfring
@ 2017-10-21 19:07 ` Stephan Mueller
2017-10-21 20:00 ` SF Markus Elfring
0 siblings, 1 reply; 5+ messages in thread
From: Stephan Mueller @ 2017-10-21 19:07 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-crypto, David S. Miller, Herbert Xu, LKML, kernel-janitors
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: crypto-testmgr: Use common error handling code in drbg_cavs_test()
2017-10-21 19:07 ` Stephan Mueller
@ 2017-10-21 20:00 ` SF Markus Elfring
0 siblings, 0 replies; 5+ messages in thread
From: SF Markus Elfring @ 2017-10-21 20:00 UTC (permalink / raw)
To: Stephan Müller, linux-crypto
Cc: David S. Miller, Herbert Xu, LKML, kernel-janitors
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-21 20:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-21 17:52 [PATCH 0/2] crypto-testmgr: Fine-tuning for drbg_cavs_test() SF Markus Elfring
2017-10-21 17:53 ` [PATCH 1/2] crypto-testmgr: Use common error handling code in drbg_cavs_test() SF Markus Elfring
2017-10-21 19:07 ` Stephan Mueller
2017-10-21 20:00 ` SF Markus Elfring
2017-10-21 17:55 ` [PATCH 2/2] crypto-testmgr: Delete an unnecessary variable initialisation " SF Markus Elfring
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).