diff mbox

enic: cleanup vic_provinfo_alloc()

Message ID 20100610075903.GL5483@bicker
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Dan Carpenter June 10, 2010, 7:59 a.m. UTC
If oui were a null variable then vic_provinfo_alloc() would leak memory.
But this function is only called from one place and oui is not null so 
I removed the check.

I also moved the memory allocation down a line so it was easier to spot.
(No one ever reads variable declarations).

Signed-off-by: Dan Carpenter <error27@gmail.com>

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

Comments

Walter Harms June 10, 2010, 9:22 a.m. UTC | #1
Dan Carpenter schrieb:
> If oui were a null variable then vic_provinfo_alloc() would leak memory.
> But this function is only called from one place and oui is not null so 
> I removed the check.
> 
> I also moved the memory allocation down a line so it was easier to spot.
> (No one ever reads variable declarations).
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/drivers/net/enic/vnic_vic.c b/drivers/net/enic/vnic_vic.c
> index d769772..0a35085 100644
> --- a/drivers/net/enic/vnic_vic.c
> +++ b/drivers/net/enic/vnic_vic.c
> @@ -25,9 +25,10 @@
>  
>  struct vic_provinfo *vic_provinfo_alloc(gfp_t flags, u8 *oui, u8 type)
>  {
> -	struct vic_provinfo *vp = kzalloc(VIC_PROVINFO_MAX_DATA, flags);
> +	struct vic_provinfo *vp;
>  
> -	if (!vp || !oui)
> +	vp = kzalloc(VIC_PROVINFO_MAX_DATA, flags);
> +	if (!vp)
>  		return NULL;
>  
>  	memcpy(vp->oui, oui, sizeof(vp->oui));
> --


This looks like memdup() ?

re,
 wh
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter June 10, 2010, 9:32 a.m. UTC | #2
On Thu, Jun 10, 2010 at 11:22:49AM +0200, walter harms wrote:
> > +	vp = kzalloc(VIC_PROVINFO_MAX_DATA, flags);
> > +	if (!vp)
> >  		return NULL;
> >  
> >  	memcpy(vp->oui, oui, sizeof(vp->oui));
> > --
> 
> 
> This looks like memdup() ?
> 

No.  VIC_PROVINFO_MAX_DATA is larger than sizeof(vp->oui).

regards,
dan carpenter

> re,
>  wh
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Feldman June 12, 2010, 12:05 a.m. UTC | #3
On 6/10/10 12:59 AM, "Dan Carpenter" <error27@gmail.com> wrote:

> If oui were a null variable then vic_provinfo_alloc() would leak memory.
> But this function is only called from one place and oui is not null so
> I removed the check.
> 
> I also moved the memory allocation down a line so it was easier to spot.
> (No one ever reads variable declarations).
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

We'll pick this one up and resubmit with the next enic patch bomb.  Thanks
Dan.

-scott

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 12, 2010, 1:37 a.m. UTC | #4
From: Dan Carpenter <error27@gmail.com>
Date: Thu, 10 Jun 2010 09:59:03 +0200

> If oui were a null variable then vic_provinfo_alloc() would leak memory.
> But this function is only called from one place and oui is not null so 
> I removed the check.
> 
> I also moved the memory allocation down a line so it was easier to spot.
> (No one ever reads variable declarations).
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Applied to net-next-2.6, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/enic/vnic_vic.c b/drivers/net/enic/vnic_vic.c
index d769772..0a35085 100644
--- a/drivers/net/enic/vnic_vic.c
+++ b/drivers/net/enic/vnic_vic.c
@@ -25,9 +25,10 @@ 
 
 struct vic_provinfo *vic_provinfo_alloc(gfp_t flags, u8 *oui, u8 type)
 {
-	struct vic_provinfo *vp = kzalloc(VIC_PROVINFO_MAX_DATA, flags);
+	struct vic_provinfo *vp;
 
-	if (!vp || !oui)
+	vp = kzalloc(VIC_PROVINFO_MAX_DATA, flags);
+	if (!vp)
 		return NULL;
 
 	memcpy(vp->oui, oui, sizeof(vp->oui));