* [PATCH] crypto: n2_core: insert '!err' condition in else scope
@ 2013-10-19 13:09 Geyslan G. Bem
2013-10-19 22:38 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan G. Bem @ 2013-10-19 13:09 UTC (permalink / raw)
To: kernel-br
Cc: Geyslan G. Bem, Herbert Xu, David S. Miller, open list:CRYPTO API,
open list
This patch moves the '!err' condition into the above else scope,
what is more obvious and has the secondary goal of avoid false-positives
in statical analyze tools.
Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
drivers/crypto/n2_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index e1f0ab4..afc6983 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -1511,9 +1511,9 @@ static int __n2_register_one_ahash(const struct n2_hash_tmpl *tmpl)
kfree(p);
} else {
pr_info("%s alg registered\n", base->cra_name);
+ if (p->hmac_type != AUTH_TYPE_RESERVED)
+ err = __n2_register_one_hmac(p);
}
- if (!err && p->hmac_type != AUTH_TYPE_RESERVED)
- err = __n2_register_one_hmac(p);
return err;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: n2_core: insert '!err' condition in else scope
2013-10-19 13:09 [PATCH] crypto: n2_core: insert '!err' condition in else scope Geyslan G. Bem
@ 2013-10-19 22:38 ` David Miller
2013-10-19 22:42 ` Geyslan Gregório Bem
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2013-10-19 22:38 UTC (permalink / raw)
To: geyslan; +Cc: kernel-br, herbert, linux-crypto, linux-kernel
From: "Geyslan G. Bem" <geyslan@gmail.com>
Date: Sat, 19 Oct 2013 10:09:31 -0300
> This patch moves the '!err' condition into the above else scope,
> what is more obvious and has the secondary goal of avoid false-positives
> in statical analyze tools.
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
I do not think patches should be applied to satisfy tools if the
code is semantically correct.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: n2_core: insert '!err' condition in else scope
2013-10-19 22:38 ` David Miller
@ 2013-10-19 22:42 ` Geyslan Gregório Bem
2013-10-19 22:49 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-19 22:42 UTC (permalink / raw)
To: David Miller; +Cc: kernel-br, Herbert Xu, open list:CRYPTO API, LKML
2013/10/19 David Miller <davem@davemloft.net>:
> From: "Geyslan G. Bem" <geyslan@gmail.com>
> Date: Sat, 19 Oct 2013 10:09:31 -0300
>
>> This patch moves the '!err' condition into the above else scope,
>> what is more obvious and has the secondary goal of avoid false-positives
>> in statical analyze tools.
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>
> I do not think patches should be applied to satisfy tools if the
> code is semantically correct.
Yes, Miller, I agree. But as I mentioned:
".. and has as the 'secondary' goal of avoid false-positives in
statical analyze tools."
The 'primary' goal is to not repeat a condition test:
"This patch moves the '!err' condition into the above else scope, what
is more obvious ..."
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: n2_core: insert '!err' condition in else scope
2013-10-19 22:42 ` Geyslan Gregório Bem
@ 2013-10-19 22:49 ` David Miller
2013-10-19 23:13 ` Geyslan Gregório Bem
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2013-10-19 22:49 UTC (permalink / raw)
To: geyslan; +Cc: kernel-br, herbert, linux-crypto, linux-kernel
From: Geyslan Gregório Bem <geyslan@gmail.com>
Date: Sat, 19 Oct 2013 19:42:35 -0300
> 2013/10/19 David Miller <davem@davemloft.net>:
>> From: "Geyslan G. Bem" <geyslan@gmail.com>
>> Date: Sat, 19 Oct 2013 10:09:31 -0300
>>
>>> This patch moves the '!err' condition into the above else scope,
>>> what is more obvious and has the secondary goal of avoid false-positives
>>> in statical analyze tools.
>>>
>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>
>> I do not think patches should be applied to satisfy tools if the
>> code is semantically correct.
>
> Yes, Miller, I agree. But as I mentioned:
> ".. and has as the 'secondary' goal of avoid false-positives in
> statical analyze tools."
>
> The 'primary' goal is to not repeat a condition test:
> "This patch moves the '!err' condition into the above else scope, what
> is more obvious ..."
I still do not feel that this change is a net-positive. Sorry.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: n2_core: insert '!err' condition in else scope
2013-10-19 22:49 ` David Miller
@ 2013-10-19 23:13 ` Geyslan Gregório Bem
2013-10-20 12:12 ` Herbert Xu
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan Gregório Bem @ 2013-10-19 23:13 UTC (permalink / raw)
To: David Miller; +Cc: kernel-br, Herbert Xu, open list:CRYPTO API, LKML
2013/10/19 David Miller <davem@davemloft.net>:
> From: Geyslan Gregório Bem <geyslan@gmail.com>
> Date: Sat, 19 Oct 2013 19:42:35 -0300
>
>> 2013/10/19 David Miller <davem@davemloft.net>:
>>> From: "Geyslan G. Bem" <geyslan@gmail.com>
>>> Date: Sat, 19 Oct 2013 10:09:31 -0300
>>>
>>>> This patch moves the '!err' condition into the above else scope,
>>>> what is more obvious and has the secondary goal of avoid false-positives
>>>> in statical analyze tools.
>>>>
>>>> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
>>>
>>> I do not think patches should be applied to satisfy tools if the
>>> code is semantically correct.
>>
>> Yes, Miller, I agree. But as I mentioned:
>> ".. and has as the 'secondary' goal of avoid false-positives in
>> statical analyze tools."
>>
>> The 'primary' goal is to not repeat a condition test:
>> "This patch moves the '!err' condition into the above else scope, what
>> is more obvious ..."
>
> I still do not feel that this change is a net-positive. Sorry.
Miller,
I can understand you. You don't want to break your git history
(blame). But when a code is clearly more readable/understandable you
must assume that it has a huge net-positive.
The patch improves the code in three ways:
- Only one if/else;
- Not blame on static analysis tools.
- And avoid more discussion about that in the future (nobody will send
patches like this to disturb you about that part of code).
The decision is yours.
Thanks again.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-20 12:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-19 13:09 [PATCH] crypto: n2_core: insert '!err' condition in else scope Geyslan G. Bem
2013-10-19 22:38 ` David Miller
2013-10-19 22:42 ` Geyslan Gregório Bem
2013-10-19 22:49 ` David Miller
2013-10-19 23:13 ` Geyslan Gregório Bem
2013-10-20 12:12 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox