On Wed, 1 Feb 2012, devendra.aaru wrote:
> On Sun, Jan 29, 2012 at 5:39 PM, Jesper Juhl <jj@xxxxxxxxxxxxx> wrote:
> > We declare 'exact' without initializing it and then do:
> >
> > [...]
> > if (strlen(p->cru_driver_name))
> > exact = 1;
> >
> > if (priority && !exact)
> > return -EINVAL;
> >
> > [...]
> >
> > If the first 'if' is not true, then the second will test an
> > uninitialized 'exact'.
>
> not needed . as the cru_driver_name will always be present :).
If that is indeed the case, and we are guaranteed that, then it would seem
that a patch like the following would be what we want instead??
Please note that this patch is intended just for discussion, nothing else
(which is why I left out a Signed-off-by on purpose), since I've not
tested it beyond checking that it compiles, nor have I verified your claim
that cru_driver_name will always be present.
Herbert, David, any input?
Subject: [PATCH] Simplify crypto_add_alg() and crypto_alg_match()
Since cru_driver_name will always be present, the test
if (strlen(p->cru_driver_name))
exact = 1;
will always be true. So there's no need to have the test at all, we
can just unconditionally assign 1 to 'exact'.
And if 'exact' is always 1, then we'll never take the true branch of
if (priority && !exact)
return -EINVAL;
so we may as well just remove those two lines completely.
At this point we may as well just remove 'exact' entirely and just use
a hardcoded 1 in the call to crypto_alg_match().
The 'name' variable is also entirely redundant since with
cru_driver_name always being present we'll always take the first
branch in
if (strlen(p->cru_driver_name))
name = p->cru_driver_name;
else
name = p->cru_name;
and then we may as well just use 'p->cru_driver_name' itself the one
place where it is needed in the call to crypto_alg_mod_lookup().
At this point all calls to the static crypto_alg crypto_alg_match()
function pass 1 as the second argument, so there's not really any
point any more in that function having that argument, so let's remove
it and the code that tests it.
And since strlen(p->cru_driver_name) will also always be true in that
function, we can simplify the assignment to 'match'.
---
crypto/crypto_user.c | 33 ++++++++-------------------------
1 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 16f8693..7760c22 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -38,23 +38,19 @@ struct crypto_dump_info {
u16 nlmsg_flags;
};
-static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact)
+static struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p)
{
struct crypto_alg *q, *alg = NULL;
down_read(&crypto_alg_sem);
list_for_each_entry(q, &crypto_alg_list, cra_list) {
- int match = 0;
+ int match;
if ((q->cra_flags ^ p->cru_type) & p->cru_mask)
continue;
- if (strlen(p->cru_driver_name))
- match = !strcmp(q->cra_driver_name,
- p->cru_driver_name);
- else if (!exact)
- match = !strcmp(q->cra_name, p->cru_name);
+ match = !strcmp(q->cra_driver_name, p->cru_driver_name);
if (match) {
alg = q;
@@ -195,7 +191,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
if (!p->cru_driver_name)
return -EINVAL;
- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;
@@ -259,7 +255,7 @@ static int crypto_update_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
if (priority && !strlen(p->cru_driver_name))
return -EINVAL;
- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;
@@ -283,7 +279,7 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
struct crypto_alg *alg;
struct crypto_user_alg *p = nlmsg_data(nlh);
- alg = crypto_alg_match(p, 1);
+ alg = crypto_alg_match(p);
if (!alg)
return -ENOENT;
@@ -304,28 +300,15 @@ static int crypto_del_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
static int crypto_add_alg(struct sk_buff *skb, struct nlmsghdr *nlh,
struct nlattr **attrs)
{
- int exact;
- const char *name;
struct crypto_alg *alg;
struct crypto_user_alg *p = nlmsg_data(nlh);
struct nlattr *priority = attrs[CRYPTOCFGA_PRIORITY_VAL];
- if (strlen(p->cru_driver_name))
- exact = 1;
-
- if (priority && !exact)
- return -EINVAL;
-
- alg = crypto_alg_match(p, exact);
+ alg = crypto_alg_match(p);
if (alg)
return -EEXIST;
- if (strlen(p->cru_driver_name))
- name = p->cru_driver_name;
- else
- name = p->cru_name;
-
- alg = crypto_alg_mod_lookup(name, p->cru_type, p->cru_mask);
+ alg = crypto_alg_mod_lookup(p->cru_driver_name, p->cru_type, p->cru_mask);
if (IS_ERR(alg))
return PTR_ERR(alg);
--
1.7.9
--
Jesper Juhl <jj@xxxxxxxxxxxxx> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
