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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux