public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Christian Engelmayer <cengelma@gmx.at>,
	devel@driverdev.osuosl.org, florian.c.schilhabel@googlemail.com,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	paul.gortmaker@windriver.com, linuxgeek@gmail.com,
	Larry.Finger@lwfinger.net
Subject: Re: [PATCH] staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext()
Date: Wed, 7 May 2014 11:55:17 +0300	[thread overview]
Message-ID: <20140507085516.GY26890@mwanda> (raw)
In-Reply-To: <1399445747.2677.4.camel@smile.fi.intel.com>

On Wed, May 07, 2014 at 09:55:47AM +0300, Andy Shevchenko wrote:
> > @@ -1824,6 +1817,15 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
> >  	default:
> >  		return -EINVAL;
> >  	}
> > +
> > +	param_len = sizeof(struct ieee_param) + pext->key_len;
> > +	param = (struct ieee_param *)_malloc(param_len);
> 
> While you are here could you substitute _malloc by kzalloc and remove
> explicit casting and memset?
> 

Normally, that's the kind of thing we would do in a separate patch
because or the "one thing per patch" rule.  Eventually someone will do a
driver wide replacement of _malloc().  Or if the bug fixer wanted, he
could do it in this patch because it is on the same line and all so it
counts as a "minor closely related change."  Either way is fine.

Another way to say this is that since the _malloc() was there in the
original code and Christian didn't introduce it, then we shouldn't
reject his patch because of it.  This is staging code, and there are so
many problems that if you start trying to fix everything you'll just get
lost.

In my experience v2 patches are much harder to write than v1 patches.
Twice in the past few days I have messed up the subject line in my v2
patches.

regards,
dan carpenter


  reply	other threads:[~2014-05-07  8:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-01  9:45 [PATCH] staging: rtl8712: fix potential leak in r871x_wx_set_enc_ext() Christian Engelmayer
2014-05-07  6:55 ` Andy Shevchenko
2014-05-07  8:55   ` Dan Carpenter [this message]
2014-05-13  9:01     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140507085516.GY26890@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=cengelma@gmx.at \
    --cc=devel@driverdev.osuosl.org \
    --cc=florian.c.schilhabel@googlemail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxgeek@gmail.com \
    --cc=paul.gortmaker@windriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox