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

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre