drm/gem: fix not to assign error value to gem name

Commit Message

Seung-Woo Kim June 26, 2013, 1:42 a.m. UTC

From: YoungJun Cho <[e-mail]>

If idr_alloc() is failed, obj->name can be error value. Also
it cleans up duplicated flink processing code.

Signed-off-by: YoungJun Cho <[e-mail]>
Signed-off-by: Seung-Woo Kim <[e-mail]>
Signed-off-by: Kyungmin Park <[e-mail]>
---
 drivers/gpu/drm/drm_gem.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

Comments

Chris Wilson June 26, 2013, 8:55 a.m. UTC | #1

On Wed, Jun 26, 2013 at 10:42:39AM +0900, Seung-Woo Kim wrote:
> From: YoungJun Cho <[e-mail]>
> 
> If idr_alloc() is failed, obj->name can be error value. Also
> it cleans up duplicated flink processing code.

You should mention that it is a regression from
commit 2e928815c (drm: convert to idr_alloc())
 
> Signed-off-by: YoungJun Cho <[e-mail]>
> Signed-off-by: Seung-Woo Kim <[e-mail]>
> Signed-off-by: Kyungmin Park <[e-mail]>

Reviewed-by: Chris Wilson <[e-mail]>
-Chris

YoungJun Cho June 26, 2013, 9:07 a.m. UTC | #2

On Jun 26, 2013 5:56 PM, "Chris Wilson" <[e-mail]> wrote:
>
> On Wed, Jun 26, 2013 at 10:42:39AM +0900, Seung-Woo Kim wrote:
> > From: YoungJun Cho <[e-mail]>
> >
> > If idr_alloc() is failed, obj->name can be error value. Also
> > it cleans up duplicated flink processing code.
>
> You should mention that it is a regression from
> commit 2e928815c (drm: convert to idr_alloc())
>
> > Signed-off-by: YoungJun Cho <[e-mail]>
> > Signed-off-by: Seung-Woo Kim <[e-mail]>
> > Signed-off-by: Kyungmin Park <[e-mail]>
>
> Reviewed-by: Chris Wilson <[e-mail]>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
>

Thank you for comments!
I'll update commit msg.

Best regards YJ

Patch

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index cf919e3..239ef30 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -453,25 +453,21 @@  drm_gem_flink_ioctl(struct drm_device *dev, void *data,
 	spin_lock(&dev->object_name_lock);
 	if (!obj->name) {
 		ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT);
-		obj->name = ret;
-		args->name = (uint64_t) obj->name;
-		spin_unlock(&dev->object_name_lock);
-		idr_preload_end();
-
 		if (ret < 0)
 			goto err;
-		ret = 0;
+
+		obj->name = ret;
 
 		/* Allocate a reference for the name table.  */
 		drm_gem_object_reference(obj);
-	} else {
-		args->name = (uint64_t) obj->name;
-		spin_unlock(&dev->object_name_lock);
-		idr_preload_end();
-		ret = 0;
 	}
 
+	args->name = (uint64_t) obj->name;
+	ret = 0;
+
 err:
+	spin_unlock(&dev->object_name_lock);
+	idr_preload_end();
 	drm_gem_object_unreference_unlocked(obj);
 	return ret;
 }