[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20161017.110055.762862023679772858.davem@davemloft.net>
Date: Mon, 17 Oct 2016 11:00:55 -0400 (EDT)
From: David Miller <davem@...emloft.net>
To: david.lebrun@...ouvain.be
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH 2/9] ipv6: sr: add code base for control plane support
of SR-IPv6
From: David Lebrun <david.lebrun@...ouvain.be>
Date: Mon, 17 Oct 2016 16:42:23 +0200
> +static int seg6_genl_set_tunsrc(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct net *net = genl_info_net(info);
> + struct seg6_pernet_data *sdata = seg6_pernet(net);
> + struct in6_addr *val, *t_old, *t_new;
Please ordre local variables from longest to shortest line (AKA reverse
christmas tree layout).
Please audit your entire submission for this problem.
> + val = (struct in6_addr *)nla_data(info->attrs[SEG6_ATTR_DST]);
Please remove all casts from void pointers, they are completely unecessary.
Since nla_data() returns "(void *)", this applies here.
Please audit your entire submission for this problem.
> + mutex_init(&sdata->lock);
> +
> + sdata->tun_src = kzalloc(sizeof(*sdata->tun_src), GFP_KERNEL);
> + if (!sdata->tun_src) {
> + kfree(sdata);
> + return -ENOMEM;
Best not to free an object while you still hold a mutex inside of it.
Also taking the mutex makes no sense at all, this object has no global
visibility, therefore no other thread of control can operate upon it.
Powered by blists - more mailing lists