On Thursday 29 July 2010 18:06:34 Laurent Pinchart wrote:
> The media_devnode structure provides support for registering and
> unregistering character devices using a dynamic major number. Reference
> counting is handled internally, making device drivers easier to write
> without having to solve the open/disconnect race condition issue over
> and over again.
> 
> The code is based on video/v4l2-dev.c.
> 
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
>  drivers/media/Makefile        |    8 +-
>  drivers/media/media-devnode.c |  326 
> +++++++++++++++++++++++++++++++++++++++++
>  include/media/media-devnode.h |   84 +++++++++++
>  3 files changed, 416 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/media/media-devnode.c
>  create mode 100644 include/media/media-devnode.h
> 

<snip>

> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> new file mode 100644
> index 0000000..6f5558c
> --- /dev/null
> +++ b/drivers/media/media-devnode.c

<snip>

> +/* Override for the open function */
> +static int media_open(struct inode *inode, struct file *filp)
> +{
> +     struct media_devnode *mdev;
> +     int ret;
> +
> +     /* Check if the media device is available. This needs to be done with
> +      * the media_devnode_lock held to prevent an open/unregister race:
> +      * without the lock, the device could be unregistered and freed between
> +      * the media_devnode_is_registered() and get_device() calls, leading to
> +      * a crash.
> +      */
> +     mutex_lock(&media_devnode_lock);
> +     mdev = container_of(inode->i_cdev, struct media_devnode, cdev);
> +     /* return ENODEV if the media device has been removed
> +        already or if it is not registered anymore. */
> +     if (!media_devnode_is_registered(mdev)) {
> +             mutex_unlock(&media_devnode_lock);
> +             return -ENODEV;
> +     }
> +     /* and increase the device refcount */
> +     get_device(&mdev->dev);
> +     mutex_unlock(&media_devnode_lock);
> +     if (mdev->fops->open) {
> +             ret = mdev->fops->open(filp);
> +             if (ret) {
> +                     put_device(&mdev->dev);
> +                     return ret;
> +             }
> +     }
> +
> +     filp->private_data = mdev;

This line should be moved up to before the if: that way the open op can rely
on private_data being setup correctly.

> +     return 0;
> +}

<snip>

Regards,

        Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to