netdev - Re: [PATCH 2/2] ipvs: generalize app registration in netns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.1207122301360.1831@ja.ssi.bg>
Date:	Thu, 12 Jul 2012 23:04:04 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Pablo Neira Ayuso <pablo@...filter.org>
cc:	Simon Horman <horms@...ge.net.au>, lvs-devel@...r.kernel.org,
	netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
	Wensong Zhang <wensong@...ux-vs.org>,
	Hans Schillstrom <hans.schillstrom@...csson.com>,
	Jesper Dangaard Brouer <brouer@...hat.com>
Subject: Re: [PATCH 2/2] ipvs: generalize app registration in netns


	Hello,

On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote:

> > +struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app)
> >  {
> >  	struct netns_ipvs *ipvs = net_ipvs(net);
> > -	/* increase the module use count */
> > -	ip_vs_use_count_inc();
> > +	struct ip_vs_app *a;
> > +	int err = 0;
> > +
> > +	if (!ipvs)
> > +		return ERR_PTR(-ENOENT);
> >  
> >  	mutex_lock(&__ip_vs_app_mutex);
> >  
> > -	list_add(&app->a_list, &ipvs->app_list);
> > +	list_for_each_entry(a, &ipvs->app_list, a_list) {
> > +		if (!strcmp(app->name, a->name)) {
> > +			err = -EEXIST;
> > +			break;
> > +		}
> > +	}
> > +	if (!err) {
> > +		a = kmemdup(app, sizeof(*app), GFP_KERNEL);
> > +		if (!a)
> > +			err = -ENOMEM;
> > +	}
> > +	if (!err) {
> > +		INIT_LIST_HEAD(&a->incs_list);
> > +		list_add(&a->a_list, &ipvs->app_list);
> > +		/* increase the module use count */
> > +		ip_vs_use_count_inc();
> > +	}
> 
> I think this code will look better if you use something like:
> 
> +		if (!strcmp(app->name, a->name)) {
> +			err = -EEXIST;
> +			goto err_unlock;
> +		}
> 
> err_unlock:
>         mutex_unlock(...)
> 
> >  
> >  	mutex_unlock(&__ip_vs_app_mutex);
> >  
> > -	return 0;
> > +	if (err)
> > +		return ERR_PTR(err);
> > +	return a;
> 
> For this three lines above, you can use:
> 
> return err ? return ERR_PTR(err) : a;

	Good point, sending v2 ...

Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html