On Fri, Jan 03, 2014 at 02:07:47PM +0200, Mika Kuoppala wrote:
> Ben Widawsky <benjamin.widaw...@intel.com> writes:
> 
> > It makes all the code which calls into this function way too confusing.
> >
> > v2: Fix destroy IOCTL as well
> >
> > v3: Clarify the other two callers of i915_gem_context_get() to never
> > check for NULL. (Mika)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72903
> > Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> 
> Reviewed-by: Mika Kuoppala <mika.kuopp...@intel.com>

Neither patch subject nor the commit message mention that this fixes a
bug. Also, the Testcase: line at the bottom is missing. I've fixed this
up.
-Daniel

> 
> >  drivers/gpu/drm/i915/i915_gem_context.c    | 12 +++++++++---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> > b/drivers/gpu/drm/i915/i915_gem_context.c
> > index ebe0f67..44dddc00 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -526,10 +526,16 @@ void i915_gem_context_close(struct drm_device *dev, 
> > struct drm_file *file)
> >  struct i915_hw_context *
> >  i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
> >  {
> > +   struct i915_hw_context *ctx;
> > +
> >     if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev))
> >             return file_priv->private_default_ctx;
> >  
> > -   return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
> > +   ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
> > +   if (!ctx)
> > +           return ERR_PTR(-ENOENT);
> > +
> > +   return ctx;
> >  }
> >  
> >  static inline int
> > @@ -776,9 +782,9 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
> > *dev, void *data,
> >             return ret;
> >  
> >     ctx = i915_gem_context_get(file_priv, args->ctx_id);
> > -   if (!ctx) {
> > +   if (IS_ERR(ctx)) {
> >             mutex_unlock(&dev->struct_mutex);
> > -           return -ENOENT;
> > +           return PTR_ERR(ctx);
> >     }
> >  
> >     idr_remove(&ctx->file_priv->context_idr, ctx->id);
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index bbff8f9..c30a6ee 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -916,7 +916,7 @@ i915_gem_validate_context(struct drm_device *dev, 
> > struct drm_file *file,
> >             return ERR_PTR(-EINVAL);
> >  
> >     ctx = i915_gem_context_get(file->driver_priv, ctx_id);
> > -   if (IS_ERR_OR_NULL(ctx))
> > +   if (IS_ERR(ctx))
> >             return ctx;
> >  
> >     hs = &ctx->hang_stats;
> > @@ -1126,7 +1126,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
> > *data,
> >     }
> >  
> >     ctx = i915_gem_validate_context(dev, file, ring, ctx_id);
> > -   if (IS_ERR_OR_NULL(ctx)) {
> > +   if (IS_ERR(ctx)) {
> >             mutex_unlock(&dev->struct_mutex);
> >             ret = PTR_ERR(ctx);
> >             goto pre_mutex_err;
> > -- 
> > 1.8.5.2
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to