Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper - Seung-Woo Kim

From: Seung-Woo Kim <sw0312.kim@samsung.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	dri-devel@lists.freedesktop.org, airlied@linux.ie,
	kyungmin.park@samsung.com, yj44.cho@samsung.com,
	Seung-Woo Kim <sw0312.kim@samsung.com>
Subject: Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper
Date: Mon, 01 Jul 2013 20:14:42 +0900	[thread overview]
Message-ID: <51D164A2.7030105@samsung.com> (raw)
In-Reply-To: <20130701105705.GG15843@cantiga.alporthouse.com>

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().

> 
> put_pid(file_priv->pid);

Yes, you are rignt. put_pid is also needed.

After discussion about above part, I'll post v3 for this.

Thanks and Regards,
- Seung-Woo Kim

> 
> after out_free.
> -Chris
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel