2007-02-24 07:44:10

by Cyrill Gorcunov

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

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

Signed-off-by: Cyrill Gorcunov <[email protected]>

---

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)


2007-02-24 12:37:19

by Oleg Verych

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

> 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?

____

2007-02-24 14:41:44

by Cyrill Gorcunov

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

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

2007-02-25 00:46:43

by Pete Zaitcev

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

On Sat, 24 Feb 2007 10:41:15 +0300, Cyrill Gorcunov <[email protected]> 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