Hi.
On 04/30/2010 06:26 PM, Luis R. Rodriguez wrote:
> On Thu, Apr 29, 2010 at 8:05 AM, matthias <mensch0815@xxxxxxxxxxxxxx> wrote:
>> +static irqreturn_t adis_irq_thread(int irq, void *dev_id)
>> +{
>> + struct spi_adis16255_data *spiadis = dev_id;
>> + int status;
>> + u16 value;
>
> u16 is unitialized, set it to 0 even though rbuf will be properly set
> only if the return status is 0.
Why? This will kill proper warnings when some path later forgets to call
spi_adis16255_read_data(). A good compiler should see the value is
initialized.
>> + /* delete error and new data bit */
>> + value = value & 0x3fff;
>> + /* set negative value */
>> + if (value & 0x2000)
>> + value = value | 0xe000;
>> +
>> + if (likely(spiadis->negative))
>> + value = -value;
>> +
>> + spiadis->data = (s16) value;
>
> There's no locking around this guy, this seems wrong.
(Or at least make it atomic_t.)
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
