netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] enic: cleanup vic_provinfo_alloc()
@ 2010-06-10  7:59 Dan Carpenter
  2010-06-10  9:22 ` walter harms
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-06-10  7:59 UTC (permalink / raw)
  To: Scott Feldman; +Cc: Vasanthy Kolluri, Roopa Prabhu, netdev, kernel-janitors

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));

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch] enic: cleanup vic_provinfo_alloc()
  2010-06-10  7:59 [patch] enic: cleanup vic_provinfo_alloc() Dan Carpenter
@ 2010-06-10  9:22 ` walter harms
  2010-06-10  9:32   ` Dan Carpenter
  2010-06-12  0:05 ` Scott Feldman
  2010-06-12  1:37 ` David Miller
  2 siblings, 1 reply; 5+ messages in thread
From: walter harms @ 2010-06-10  9:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Scott Feldman, Vasanthy Kolluri, Roopa Prabhu, netdev,
	kernel-janitors



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] enic: cleanup vic_provinfo_alloc()
  2010-06-10  9:22 ` walter harms
@ 2010-06-10  9:32   ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-06-10  9:32 UTC (permalink / raw)
  To: walter harms
  Cc: Scott Feldman, Vasanthy Kolluri, Roopa Prabhu, netdev,
	kernel-janitors

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] enic: cleanup vic_provinfo_alloc()
  2010-06-10  7:59 [patch] enic: cleanup vic_provinfo_alloc() Dan Carpenter
  2010-06-10  9:22 ` walter harms
@ 2010-06-12  0:05 ` Scott Feldman
  2010-06-12  1:37 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: Scott Feldman @ 2010-06-12  0:05 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Vasanthy Kolluri, Roopa Prabhu, netdev, kernel-janitors

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] enic: cleanup vic_provinfo_alloc()
  2010-06-10  7:59 [patch] enic: cleanup vic_provinfo_alloc() Dan Carpenter
  2010-06-10  9:22 ` walter harms
  2010-06-12  0:05 ` Scott Feldman
@ 2010-06-12  1:37 ` David Miller
  2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-06-12  1:37 UTC (permalink / raw)
  To: error27; +Cc: scofeldm, vkolluri, roprabhu, netdev, kernel-janitors

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-06-12  1:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-10  7:59 [patch] enic: cleanup vic_provinfo_alloc() Dan Carpenter
2010-06-10  9:22 ` walter harms
2010-06-10  9:32   ` Dan Carpenter
2010-06-12  0:05 ` Scott Feldman
2010-06-12  1:37 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).