[PATCH] USB Elan FTDI: check for workqueue creation v2

[PATCH] USB Elan FTDI: check for workqueue creation v2

[Cyrill Gorcunov]

This patch prevents from NULL pointer usage if
workqueue creation failed.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

---

Btw, Pete, you are right! C99 ANSI standart says that static pointer
if it not initialized explicitly has to be set to NULL by compiler ;)
Thanks a lot for comments and Ack the patch please.

 drivers/usb/misc/ftdi-elan.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/ftdi-elan.c b/drivers/usb/misc/ftdi-elan.c
index 0c1d66d..bc3327e 100644
--- a/drivers/usb/misc/ftdi-elan.c
+++ b/drivers/usb/misc/ftdi-elan.c
@@ -2905,17 +2905,31 @@ static int __init ftdi_elan_init(void)
 {
         int result;
         printk(KERN_INFO "driver %s built at %s on %s\n", ftdi_elan_driver.name,
-                 __TIME__, __DATE__);
+	       __TIME__, __DATE__);
         init_MUTEX(&ftdi_module_lock);
         INIT_LIST_HEAD(&ftdi_static_list);
         status_queue = create_singlethread_workqueue("ftdi-status-control");
+	if (!status_queue)
+		goto err1;
         command_queue = create_singlethread_workqueue("ftdi-command-engine");
+	if (!command_queue)
+		goto err2;
         respond_queue = create_singlethread_workqueue("ftdi-respond-engine");
+	if (!respond_queue)
+		goto err3;
         result = usb_register(&ftdi_elan_driver);
         if (result)
                 printk(KERN_ERR "usb_register failed. Error number %d\n",
-                        result);
+		       result);
         return result;
+
+ err3:
+	destroy_workqueue(command_queue);
+ err2:
+	destroy_workqueue(status_queue);
+ err1:
+	printk(KERN_ERR "%s couldn't create workqueue\n", ftdi_elan_driver.name);
+	return -ENOMEM;
 }
 
 static void __exit ftdi_elan_exit(void)

Re: [PATCH] USB Elan FTDI: check for workqueue creation v2

[Oleg Verych]

> From: Cyrill Gorcunov
> Newsgroups: gmane.linux.kernel
> Subject: bss zeroing ([PATCH] USB Elan FTDI: check for workqueue creation v2)
> Date: Sat, 24 Feb 2007 10:41:15 +0300
[]
> Btw, Pete, you are right! C99 ANSI standart says that static pointer
> if it not initialized explicitly has to be set to NULL by compiler ;)
> Thanks a lot for comments and Ack the patch please.

Are you sure about "by compiler"? I mean, why not by OS, in case of uCs
by code/data memory image generator?

____

Re: [PATCH] USB Elan FTDI: check for workqueue creation v2

[Cyrill Gorcunov]

On Sat, Feb 24, 2007 at 01:46:44PM +0100, Oleg Verych wrote:
| > From: Cyrill Gorcunov
| > Newsgroups: gmane.linux.kernel
| > Subject: bss zeroing ([PATCH] USB Elan FTDI: check for workqueue creation v2)
| > Date: Sat, 24 Feb 2007 10:41:15 +0300
| []
| > Btw, Pete, you are right! C99 ANSI standart says that static pointer
| > if it not initialized explicitly has to be set to NULL by compiler ;)
| > Thanks a lot for comments and Ack the patch please.
| 
| Are you sure about "by compiler"? I mean, why not by OS, in case of uCs
| by code/data memory image generator?
| 
| ____
| 

I don't think that is the point... C compiler may put that data to a
special section of image (which is to be filled by zero) or add a few
explicit instructions... I don't care the method for a while, I need
the result ;)

		Cyrill

Re: [PATCH] USB Elan FTDI: check for workqueue creation v2

[Pete Zaitcev]

On Sat, 24 Feb 2007 10:41:15 +0300, Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> Thanks a lot for comments and Ack the patch please.

Cyrill, I forgot to mention a couple of points, sorry.

>          printk(KERN_INFO "driver %s built at %s on %s\n", ftdi_elan_driver.name,
> -                 __TIME__, __DATE__);
> +	       __TIME__, __DATE__);

Is this really necessary?

>          respond_queue = create_singlethread_workqueue("ftdi-respond-engine");
> +	if (!respond_queue)
> +		goto err3;

This is good, but...

>          result = usb_register(&ftdi_elan_driver);
>          if (result)
>                  printk(KERN_ERR "usb_register failed. Error number %d\n",
> -                        result);
> +		       result);
>          return result;

What does happen if usb_register fails? Since you fixed the
create_singlethread_workqueue, why not this too?

-- Pete
cc: to Tony@ELAN and linux-usb-devel