On 04/29/2010 10:35 AM, matthias wrote:
> 2010/4/29 Jiri Slaby <jirislaby@xxxxxxxxx>:
>> On 04/28/2010 08:45 PM, matthias wrote:
>>> --- /dev/null
>>> +++ b/drivers/staging/adis16255/adis16255.c
>>> +static int spi_adis16255_read_data(struct spi_adis16255_data *spiadis,
>>> + u8 adr,
>>> + u8 *rbuf)
>>> +{
>>> + struct spi_device *spi = spiadis->spi;
>>> + struct spi_message msg;
>>> + struct spi_transfer xfer1, xfer2;
>>> + u8 *buf, *rx;
>>> + int ret;
>>> +
>>> + buf = kmalloc(4, GFP_KERNEL);
>>> + if (!buf)
>>> + return -ENOMEM;
>>> +
>>> + rx = kzalloc(4, GFP_KERNEL);
>>> + if (!rx)
>>> + return -ENOMEM;
>>
>> buf leaks here. You can just use buffers on stack for 8 bytes (unless it
>> DMAs there).
>
> I'm not sure if I understand you right. A buffer can not be put on the
> stack if it is not multiple of 8. Why? (sorry, maybe off topic here)
No, I meant you alloc 4+4 bytes which can be safely be on stack. UNLESS
the SPI layer does DMA to/from that memory. In that case you should
better handle failures (a leak) in this code.
>>> + schedule_work(&spiadis->wq);
>>
>> So this can be threaded irq with NULL handler, no?
>
> Why? I request the interrupt in the probe function, passing spiadis struct.
> Should I check for spiadis, for defensive programming sake?
You know what is threaded irq, don't you? You just request_threaded_irq
with NULL for irq handler and directly spi_workqueue as a thread
function (with changed prototype). It will save you from messing up with
workqueues.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
