From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S965062AbbBJOmi (ORCPT <rfc822;w@1wt.eu>);
	Tue, 10 Feb 2015 09:42:38 -0500
Received: from smtprelay0046.hostedemail.com ([216.40.44.46]:34946 "EHLO
	smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL)
	by vger.kernel.org with ESMTP id S1756369AbbBJOmM (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 10 Feb 2015 09:42:12 -0500
X-Session-Marker: 6A6F6540706572636865732E636F6D
X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::,RULES_HIT:41:355:379:541:599:966:979:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1541:1593:1594:1711:1730:1747:1777:1792:2196:2199:2393:2559:2562:2828:2902:3138:3139:3140:3141:3142:3353:3622:3865:3866:3867:3870:3872:4321:4383:4385:4395:5007:6261:7974:10004:10400:10848:11026:11232:11473:11658:11914:12043:12296:12517:12519:12740:13069:13161:13229:13311:13357:14096:14097:21067:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0
X-HE-Tag: cart00_2ec9b2f5d6839
X-Filterd-Recvd-Size: 2695
Message-ID: <1423579319.8339.1.camel@perches.com>
Subject: Re: [PATCH v2 1/1] staging: ozwpan: Remove allocation from
 delaration line
From: Joe Perches <joe@perches.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Quentin Lambert <lambert.quentin@gmail.com>,
        Shigekatsu Tateno <shigekatsu.tateno@atmel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        devel@driverdev.osuosl.org, kernel-janitors@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date: Tue, 10 Feb 2015 06:41:59 -0800
In-Reply-To: <20150210101312.GG5206@mwanda>
References: <20150210092412.GA12753@sloth> <20150210101312.GG5206@mwanda>
Content-Type: text/plain; charset="ISO-8859-1"
X-Mailer: Evolution 3.12.7-0ubuntu1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, 2015-02-10 at 13:13 +0300, Dan Carpenter wrote:
> > diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
[]
> > @@ -280,8 +280,9 @@ static void oz_free_urb_link(struct oz_urb_link *urbl)
> >   */
> >  static struct oz_endpoint *oz_ep_alloc(int buffer_size, gfp_t mem_flags)
> >  {
> > -	struct oz_endpoint *ep =
> > -		kzalloc(sizeof(struct oz_endpoint)+buffer_size, mem_flags);
> > +	struct oz_endpoint *ep;
> > +
> > +	ep = kzalloc(sizeof(struct oz_endpoint)+buffer_size, mem_flags);
> >  	if (ep) {
> 
> Also notice how in the original code, we had to mangle the code to make
> it fit into 80 characters so the new code looks much better.

or maybe
	ep = kzalloc(sizeof(*ep) + buffer_size, mem_flags);

The current function also tests the return of ep slightly backwards.

static struct oz_endpoint *oz_ep_alloc(int buffer_size, gfp_t mem_flags)
{
	struct oz_endpoint *ep =
		kzalloc(sizeof(struct oz_endpoint)+buffer_size, mem_flags);
	if (ep) {
		INIT_LIST_HEAD(&ep->urb_list);
		INIT_LIST_HEAD(&ep->link);
		ep->credit = -1;
		if (buffer_size) {
			ep->buffer_size = buffer_size;
			ep->buffer = (u8 *)(ep+1);
		}
	}
	return ep;
}

Perhaps more typical would be:

static struct oz_endpoint *oz_ep_alloc(int buffer_size, gfp_t mem_flags)
{
	struct oz_endpoint *ep;

	ep = kzalloc(sizeof(*ep) + buffer_size, mem_flags);
	if (!ep)
		return NULL;

	INIT_LIST_HEAD(&ep->urb_list);
	INIT_LIST_HEAD(&ep->link);
	ep->credit = -1;
	if (buffer_size) {
		ep->buffer_size = buffer_size;
		ep->buffer = (u8 *)(ep + 1);
	}

	return ep;
}

Maybe buffer_size should be size_t too to avoid
possible negative values.