Re: [PATCH 1/2] staging: iio replaced kmalloc with local variables.

[Greg KH]

On Mon, Jun 06, 2011 at 03:28:29PM -0700, Joe Perches wrote:
> On Mon, 2011-06-06 at 15:21 -0700, Greg KH wrote:
> > On Mon, Jun 06, 2011 at 03:10:57PM -0700, Joe Perches wrote:
> > > On Mon, 2011-06-06 at 14:55 -0700, Greg KH wrote:
> > > > On Tue, Jun 07, 2011 at 12:37:37AM +0530, anish wrote:
> > > > > From: anish kumar <anish198519851985@xxxxxxxxx>
> > > > > Replace kmalloc with local variables as it was un-necessary and
> > > > > also removed the redudant code after this change.
> > > > SPI data, like USB data, has to come from kmalloced data, not from the
> > > > stack, or bad things can, and will, happen.
> > > Perhaps just add a comment like:
> > > +       u8 *tx = kmalloc(2, GFP_KERNEL);	/* can't be on stack */
> > You really want to do to that for _EVERY_ SPI and USB driver?  I don't
> > think so.
> 
> Nope, only the ones that look especially odd because 
> 	kmalloc(sizeof(struct foo), ...)
> or
> 	kmalloc(sizeof("type), ...)
> is not used.
> 
> It might be better to just declare a 2 byte struct.

No, this is a very common thing for all USB and SPI drivers.  It's so
obvious that once I saw the Subject: line, I knew this patch was going
to be wrong.

This is something that the USB and SPI developers know all about, it's
the way things work, and this driver works, so why are people trying to
"clean" it up in ways that will break it, or cause extra work with
structures where they are not needed at all?

odd.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/