hw_random: cleanup in hwrng_register()
Subsystems (2)
PATCH 1/1 hw_random: cleanup in hwrng_register()
My static checker complains that:
drivers/char/hw_random/core.c:341 hwrng_register()
warn: we tested 'old_rng' before and it was 'false'
The problem is that sometimes we test "if (!old_rng)" and sometimes we test "if (must_register_misc)". The static checker knows they are equivalent but a human being reading the code could easily be confused.
I have simplified the code by removing the "must_register_misc" variable and I have removed the redundant check on "if (!old_rng)".
drivers/char/hw_random/core.c +3 -7
@@ -301,7 +301,6 @@ err_misc_dereg:int hwrng_register (struct hwrng *rng) { -int must_register_misc;int err = -EINVAL ;struct hwrng *old_rng, *tmp;
@@ -326,7 +325,6 @@ int hwrng_register(struct hwrng *rng) goto out_unlock; } - must_register_misc = (current_rng =NULL ); old_rng = current_rng;if (!old_rng) { err =hwrng_init (rng);
@@ -335,13 +333,11 @@ int hwrng_register(struct hwrng *rng) current_rng = rng; } err =0 ; -if (must_register_misc) { +if (!old_rng) { err =register_miscdev ();if (err) { -if (!old_rng) { -hwrng_cleanup (rng); - current_rng =NULL ; - } +hwrng_cleanup (rng); + current_rng =NULL ; goto out_unlock; } }
Quoted 5 lines · Dan Carpenter
My static checker complains that:
drivers/char/hw_random/core.c:341hwrng_register()
warn: we tested 'old_rng' before and it was 'false'
Quoted 8 lines
The problem is that sometimes we test "if (!old_rng)" and sometimes we test "if (
must_register_misc)". The static checker knows they are equivalent but a human being reading the code could easily be confused.I have simplified the code by removing the "
must_register_misc" variable and I have removed the redundant check on "if (!old_rng)".Signed-off-by Dan Carpenter <[email protected]>
Yeah, clearer too.
Thanks,
Rusty.
Quoted 38 lines
drivers/char/hw_random/core.c +3
-7@@ -301,7 +301,6 @@ err_misc_dereg:int hwrng_register (struct hwrng *rng) { -int must_register_misc;int err = -EINVAL ;struct hwrng *old_rng, *tmp;@@ -326,7 +325,6 @@ int hwrng_register(struct hwrng *rng) goto out_unlock; } - must_register_misc = (current_rng =NULL ); old_rng = current_rng;if (!old_rng) { err =hwrng_init (rng);@@ -335,13 +333,11 @@ int hwrng_register(struct hwrng *rng) current_rng = rng; } err =0 ; -if (must_register_misc) { +if (!old_rng) { err =register_miscdev ();if (err) { -if (!old_rng) { -hwrng_cleanup (rng); - current_rng =NULL ; - } +hwrng_cleanup (rng); + current_rng =NULL ; goto out_unlock; } }
Herbert Xu
maintainerQuoted 10 lines · Rusty Russell
Quoted 9 lines · Dan Carpenter
My static checker complains that:
drivers/char/hw_random/core.c:341hwrng_register()
warn: we tested 'old_rng' before and it was 'false'The problem is that sometimes we test "if (!old_rng)" and sometimes we test "if (
must_register_misc)". The static checker knows they are equivalent but a human being reading the code could easily be confused.
Quoted 8 lines
Quoted 4 lines
I have simplified the code by removing the "
must_register_misc" variable and I have removed the redundant check on "if (!old_rng)".Signed-off-by Dan Carpenter <[email protected]>Yeah, clearer too.
Reviewed-by Rusty Russell <[email protected]>
Patch applied.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt