Thanks for the comments.
On Fri, May 18, 2001 at 10:05:26AM +0200, Oliver Neukum wrote:
>
> 1)
> in open:
>
> if ((subminor < 0) ||
> (subminor >= MAX_DEVICES)) {
> skel = minor_table[subminor];
>
> Isn't there a ! missing ?
Yes it is, thanks, now fixed.
> 2)
> in open:
>
> You increase the module use count after doing a down()
> This is a race condition.
> Also it is typical, in an example it is serious.
now fixed.
> 3)
> in release:
>
> in the already unplugged case you don't decrease the module use count
now fixed.
> 4)
> in write:
>
> if submitting the URB fails, bytes_written is returned, although they were
> not written.
now fixed.
> 5)
> in write:
>
> copy_from_user is not checked for -EFAULT
now fixed.
>
> 6)
> in ioctl:
>
> The ioctl handler is called directly.
> The correct default return is -ENOTTY
now fixed, I've spend too much time in tty drivers :)
> 7)
> in write:
>
> you release the lock before the URB is completed.
> This is deadly.
It is if you want to use it to show completion of the urb. You mentioned
holding the semaphore until the completion function is called as one way
to solve the urb->status problem in a different message. I think I'll
incorporate this, or just add a "urb complete" flag in the structure to
solve the race condition.
Thanks again for the comments.
greg k-h
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
