[PATCH v2 3/3] drm: fix error routines in drm_open_helper

[YoungJun Cho]

Hello Chris,

On Jul 1, 2013 8:53 PM, "Chris Wilson" <chris at chris-wilson.co.uk> wrote:
>
> On Mon, Jul 01, 2013 at 08:14:42PM +0900, Seung-Woo Kim wrote:
> > Hello Chris,
> >
> > On 2013년 07월 01일 19:57, Chris Wilson wrote:
> > > On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
> > >> +
> > >> +out_close:
> > >> +  if (dev->driver->postclose)
> > >> +          dev->driver->postclose(dev, priv);
> > >> +out_free:
> > >>    kfree(priv);
> > >>    filp->private_data = NULL;
> > >>    return ret;
> > >
> > > Looks like we are also missing:
> > >
> > > if (drm_core_check_feature(dev, DRIVER_PRIME))
> > >     drm_prime_destroy_file_private(&file_priv->prime);
> >
> > Currently, file_priv->prime is just initialized, and
> > drm_prime_destroy_file_private() just checks the list is empty and at
> > the open time, prime list is always empty. So IMHO, it seems unnecessary
> > to call drm_prime_destroy_file_private().
> >
> > If this is necessary, drm_gem_release() is also needed because the pair
> > function of drm_gem_open() is drm_gem_release().
>
> Hmm, could be a slight ordering bug in drm_release(), but yes I agree
> that we should also call drm_gem_release() here in drm_open_helper. It
> is better for the code to be symmetric in cleaning up anything that we
> create so that we are correct for future changes. We might as well fix it
> correctly, rather than having to rediscover this bug at some later date.
> -Chris
>

Thank you for quick reviews.
We'll post V3 patch with this also.

Best regards YJ

> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130701/ac7b09d9/attachment.html>