> +// prevent races between open() and disconnect()
> +static DECLARE_MUTEX(disconnect_sem);

> +static int idmouse_release(struct inode *inode, struct file *file)
> +{
> +     dev = (struct usb_idmouse *) file->private_data;
> +     if (dev == NULL)
> +             return -ENODEV;
> +     // lock our device
> +     down(&dev->sem);
> +     if (!dev->present && !dev->open) {
> +             // the device was unplugged before the file was released 
> +             up(&dev->sem);
> +             idmouse_delete(dev);
> +             return 0;

You should be taking disconnect_sem here instead. Think about it:
you are competing with an open. What happens there?

+       down(&disconnect_sem);
+       dev = usb_get_intfdata(interface);
+       if (!dev) {
+               up(&disconnect_sem);
+               return -ENODEV;
+       }
+       down(&dev->sem);
+       if (dev->open) {

Right, the open blocks at dev->open. Then once you up the semaphore
and destory the device, it checks the open flag and happily proceeds

> +     } else do {

You will burn in hell for this.

And lay off extra empty lines, please.

And since we are at it, is there a particular reason for this device driver
to be in the kernel?

-- Pete


-------------------------------------------------------
This SF.Net email is sponsored by OSTG. Have you noticed the changes on
Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now,
one more big change to announce. We are now OSTG- Open Source Technology
Group. Come see the changes on the new OSTG site. www.ostg.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to