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) {
>
>
>
>-------------------------------------------------------
>This SF.Net email is sponsored by xPML, a groundbreaking scripting language
>that extends applications into web and mobile media. Attend the live webcast
>and join the prime developer group breaking into this new coding territory!
>http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
>_______________________________________________
>linux-usb-devel@lists.sourceforge.net
>To unsubscribe, use the last form field at:
>https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
>  
>
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.


Michael Downey



-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to