On Tuesday 08 October 2002 07:40, John Tyner wrote:
> This is a resend of the patch I sent earlier. I worked on the one earlier
> while away from home and was unable to test it. This one has been fixed,
> tested, and should be correct. Sorry for the confusion.
>
> Patch is against 2.5.41.
>
> Thanks,
> John
Hi,
+ if ( waitqueue_active( &vicam_v4l_priv->no_users ) ) {
+ wake_up( &vicam_v4l_priv->no_users );
never ever do this. Always do the wake_up unconditionally.
ioctl() - VIDIOSYNC: this may hang if you unplug at the wrong moment,
as there's nobody to up the semaphore in that case.
You should do the up in the disconnect handler and check for disconnection
in the ioctl so you can return -ENODEV in that case.
in disconnect:
+ /* make sure no one will submit another urb */
+ clear_bit( 0, &vicam_v4l_priv->vicam_present );
But it does not guard against control messages in flight.
You need a semaphore to do that.
+ usb_unlink_urb( vicam_v4l_priv->urb );
+ down( &vicam_v4l_priv->busy_mutex );
This can hang for two reasons,
- you might not be the only user of that semaphore
- usb_unlink_urb may fail due to there being no queued urb,
eg. if the completion handler is running - you need to check the return
value
+ if ( vdev->users ) {
+ sleep_on( &vicam_v4l_priv->no_users );
+ }
_Very_ bad idea.
First, never use sleep_on. Use the appropriate macro.
Second, you have blocked khubd without an upper time limit.
That you _must_ _not_ in any case do.
+ kfree( vicam_v4l_priv );
+ kfree( vicam_usb_priv );
Potentionally deadly if the device is open, you need to defer it to release
in that case
Sorry for all that trouble.
Regards
Oliver