[PATCH 13/16] drm: Refactor drop/set master code a bit

[Emil Velikov]

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 at ffwll.ch> 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