Message ID | 20141016075721.GC29096@mwanda (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote: > The lock has been freed in usbvision_release() so there is no need to > call mutex_unlock() here. Yuck :-/ The driver should really be converted to use video_device::release. That might be out of scope for this fix though. Is usbvision maintained ? > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/media/usb/usbvision/usbvision-video.c > b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 > 100644 > --- a/drivers/media/usb/usbvision/usbvision-video.c > +++ b/drivers/media/usb/usbvision/usbvision-video.c > @@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file) > if (usbvision->remove_pending) { > printk(KERN_INFO "%s: Final disconnect\n", __func__); > usbvision_release(usbvision); > + return 0; > } > mutex_unlock(&usbvision->v4l2_lock); > > @@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file) > if (usbvision->remove_pending) { > printk(KERN_INFO "%s: Final disconnect\n", __func__); > usbvision_release(usbvision); > + return err_code; > } > > mutex_unlock(&usbvision->v4l2_lock);
On 10/16/2014 10:09 AM, Laurent Pinchart wrote: > On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote: >> The lock has been freed in usbvision_release() so there is no need to >> call mutex_unlock() here. > > Yuck :-/ > > The driver should really be converted to use video_device::release. That might > be out of scope for this fix though. Is usbvision maintained ? I have hardware, and at some point I plan to convert it to modern frameworks. But ENOTIME for now. So I guess I might be the closest to a being a maintainer. Regards, Hans > >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> >> diff --git a/drivers/media/usb/usbvision/usbvision-video.c >> b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 >> 100644 >> --- a/drivers/media/usb/usbvision/usbvision-video.c >> +++ b/drivers/media/usb/usbvision/usbvision-video.c >> @@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file) >> if (usbvision->remove_pending) { >> printk(KERN_INFO "%s: Final disconnect\n", __func__); >> usbvision_release(usbvision); >> + return 0; >> } >> mutex_unlock(&usbvision->v4l2_lock); >> >> @@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file) >> if (usbvision->remove_pending) { >> printk(KERN_INFO "%s: Final disconnect\n", __func__); >> usbvision_release(usbvision); >> + return err_code; >> } >> >> mutex_unlock(&usbvision->v4l2_lock); > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 16 October 2014 10:23:05 Hans Verkuil wrote: > On 10/16/2014 10:09 AM, Laurent Pinchart wrote: > > On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote: > >> The lock has been freed in usbvision_release() so there is no need to > >> call mutex_unlock() here. > > > > Yuck :-/ > > > > The driver should really be converted to use video_device::release. That > > might be out of scope for this fix though. Is usbvision maintained ? > > I have hardware, and at some point I plan to convert it to modern > frameworks. But ENOTIME for now. So I guess I might be the closest to a > being a maintainer. Can you ack the patch then ? :-) > >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > >> > >> diff --git a/drivers/media/usb/usbvision/usbvision-video.c > >> b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 > >> 100644 > >> --- a/drivers/media/usb/usbvision/usbvision-video.c > >> +++ b/drivers/media/usb/usbvision/usbvision-video.c > >> @@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file) > >> if (usbvision->remove_pending) { > >> printk(KERN_INFO "%s: Final disconnect\n", __func__); > >> usbvision_release(usbvision); > >> + return 0; > >> } > >> mutex_unlock(&usbvision->v4l2_lock); > >> > >> @@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file) > >> if (usbvision->remove_pending) { > >> printk(KERN_INFO "%s: Final disconnect\n", __func__); > >> usbvision_release(usbvision); > >> + return err_code; > >> } > >> > >> mutex_unlock(&usbvision->v4l2_lock);
diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 100644 --- a/drivers/media/usb/usbvision/usbvision-video.c +++ b/drivers/media/usb/usbvision/usbvision-video.c @@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file) if (usbvision->remove_pending) { printk(KERN_INFO "%s: Final disconnect\n", __func__); usbvision_release(usbvision); + return 0; } mutex_unlock(&usbvision->v4l2_lock); @@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file) if (usbvision->remove_pending) { printk(KERN_INFO "%s: Final disconnect\n", __func__); usbvision_release(usbvision); + return err_code; } mutex_unlock(&usbvision->v4l2_lock);
The lock has been freed in usbvision_release() so there is no need to call mutex_unlock() here. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html