Greg KH wrote:
>On Mon, Mar 27, 2006 at 09:05:38AM -0700, Michael Downey wrote:
>
>
>>Adrian Bunk wrote:
>>
>>
>>
>>>gcc reported the following:
>>>
>>><-- snip -->
>>>
>>>...
>>> CC drivers/usb/input/keyspan_remote.o
>>>drivers/usb/input/keyspan_remote.c: In function 'keyspan_irq_recv':
>>>drivers/usb/input/keyspan_remote.c:186: warning: 'message.toggle' may be
>>>used uninitialized in this function
>>>...
>>>
>>><-- snip -->
>>>
>>>
>>>gcc is right, there is an error case where this actually happens.
>>>
>>>What about this patch that returns in this error case?
>>>
>>>Signed-off-by: Darren Jenkins <[EMAIL PROTECTED]>
>>>
>>>--- linux-2.6.16-mm1-full/drivers/usb/input/keyspan_remote.c.old
>>>2006-03-25 15:44:56.000000000 +0100
>>>+++ linux-2.6.16-mm1-full/drivers/usb/input/keyspan_remote.c 2006-03-25
>>>15:45:48.000000000 +0100
>>>@@ -285,30 +285,31 @@ static void keyspan_check_data(struct us
>>> return;
>>> }
>>> }
>>>
>>> keyspan_load_tester(remote, 6);
>>> if ((remote->data.tester & ZERO_MASK) == ZERO) {
>>> message.toggle = 0;
>>> remote->data.tester = remote->data.tester >> 5;
>>> remote->data.bits_left -= 5;
>>> } else if ((remote->data.tester & ONE_MASK) == ONE) {
>>> message.toggle = 1;
>>> remote->data.tester = remote->data.tester >> 6;
>>> remote->data.bits_left -= 6;
>>> } else {
>>> err("%s - Error in message, invalid toggle.\n",
>>> __FUNCTION__);
>>>+ return;
>>> }
>>>
>>> keyspan_load_tester(remote, 5);
>>> if ((remote->data.tester & STOP_MASK) == STOP) {
>>> remote->data.tester = remote->data.tester >> 5;
>>> remote->data.bits_left -= 5;
>>> } else {
>>> err("Bad message recieved, no stop bit found.\n");
>>> }
>>>
>>> dev_dbg(&remote->udev->dev,
>>> "%s found valid message: system: %d, button: %d,
>>> toggle: %d\n",
>>> __FUNCTION__, message.system, message.button,
>>> message.toggle);
>>>
>>> if (message.toggle != remote->toggle) {
>>>
>>>
>>My reasoning for not having the return was that it allowed the driver to
>>finish reading the rest of the garbage message. It also won't reset the
>>stage on the message back to 0 which is what we want.
>>
>>So the change probably should be something like:
>>
>> } else {
>> err("%s - Error in message, invalid toggle.\n", __FUNCTION__);
>>+ message->stage = 0;
>>+ return;
>> }
>>
>>
>>That will cause the next read to start again from the beginning. Other
>>than that the change looks fine with me. I'll see if I can do some
>>testing on the change this week. I probably should do a bit more review
>>of that error case as I haven't really looked at the code in some time.
>>
>>
>
>Ok, I'll drop this one and wait for a cleaned up and tested version from
>you.
>
>thanks,
>
>greg k-h
>
>
Here is the updated patch. Also this diff is against a 2.6.15 kernel but
I don't believe anything has changed in this driver for a while now. If
there is a problem let me know and I'll resend.
Signed-off-by: Michael Downey <[EMAIL PROTECTED]>
--- linux/drivers/usb/input/keyspan_remote.c.orig 2006-04-03
08:49:33.000000000 -0600
+++ linux/drivers/usb/input/keyspan_remote.c 2006-03-31 13:34:23.000000000
-0700
@@ -297,6 +297,8 @@ static void keyspan_check_data(struct us
remote->data.bits_left -= 6;
} else {
err("%s - Error in message, invalid toggle.\n",
__FUNCTION__);
+ remote->stage = 0;
+ return;
}
keyspan_load_tester(remote, 5);
