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.pinch...@ideasonboard.com>
> > ---
> > 
> >  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to