Hi Daniel,
It doesn't look quite right I'm afraid. This causes a leak plus
there's a small style issue. See below for details.
On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> @@ -134,24 +152,21 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
>
> /* take another reference for the copy in the local file priv */
> old_master = fpriv->master;
> - fpriv->master = drm_master_get(dev->master);
> + fpriv->master = drm_master_create(dev);
> + if (!fpriv->master)
> + return -ENOMEM;
>
Now dev->master != fpriv->master
> if (dev->driver->master_create) {
> ret = dev->driver->master_create(dev, fpriv->master);
> if (ret)
> goto out_err;
> }
> - if (dev->driver->master_set) {
> - ret = dev->driver->master_set(dev, fpriv, true);
> - if (ret)
> - goto out_err;
> - }
> -
> - fpriv->is_master = 1;
> fpriv->allowed_master = 1;
> fpriv->authenticated = 1;
> - if (old_master)
> - drm_master_put(&old_master);
> +
> + ret = drm_set_master(dev, fpriv, true);
> +static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
> + bool new_master)
> +{
> + int ret = 0;
> +
> + dev->master = drm_master_get(fpriv->master);
drm_master_get() is defined as follows
kref_get(&master->refcount);
return master;
Since dev->master != fpriv->master, we'll leak the former.
> +void drm_drop_master(struct drm_device *dev,
> + struct drm_file *fpriv)
Function is used only locally thus it should be static.
Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
