On Tue, 27 Mar 2007 19:14:05 +0400, Cyrill Gorcunov <[EMAIL PROTECTED]> wrote:
> --- a/drivers/usb/misc/ftdi-elan.c
> @@ -2903,7 +2903,7 @@ static struct usb_driver ftdi_elan_driver = {
> };
> static int __init ftdi_elan_init(void)
> {
> - int result;
> + int result = 0;
Why do you need this?
> @@ -2918,18 +2918,25 @@ static int __init ftdi_elan_init(void)
> if (!respond_queue)
> goto err3;
> result = usb_register(&ftdi_elan_driver);
> - if (result)
> + if (result) {
> printk(KERN_ERR "usb_register failed. Error number %d\n",
> result);
> + goto err4;
> + }
> return result;
>
> + err4:
> + destroy_workqueue(respond_queue);
> err3:
This is fine, although I do wish you wouldn't number the exception labels.
If anything is changed, someone might try to rearrange and renumber them
and that leads to bugs.
> err1:
> - printk(KERN_ERR "%s couldn't create workqueue\n",
> ftdi_elan_driver.name);
> - return -ENOMEM;
> + if (result == 0) {
> + result = -ENOMEM;
> + printk(KERN_ERR "%s couldn't create workqueue\n",
> ftdi_elan_driver.name);
> + }
> + return result;
What in the world is this supposed to do? Under what conditions can
result be zero here?
Personally, I would get rid of the printk. If your modprobe fails,
it's a good enough indication. Or at least, change the text to
something more neutral, like "unable to initialize (%d)" and print
the error code. It's not just about workqueues now.
-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
