Hi Hans,
On Sunday 01 August 2010 13:14:18 Hans Verkuil wrote:
> 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 <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> >
> > 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.
Agreed, I'll fix that.
> > + return 0;
> > +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
