Re: [PATCH 2/4] drm/prime: cleanup goto mess in drm_gem_prime_handle_to_fd

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Oct 15, 2015 at 11:51:39AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> I think I might have been responsible for some of this, but it's messy
> to decode, this doesn't change anything, but cleans up the goto's
> and exit paths.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Before coffee, but looks good. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_prime.c | 73 ++++++++++++++++++++-------------------------
>  1 file changed, 32 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 9f935f5..d22ce83 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -420,47 +420,42 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  	dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle);
>  	if (dmabuf) {
>  		get_dma_buf(dmabuf);
> -		goto out_have_handle;
> -	}
> -
> -	mutex_lock(&dev->object_name_lock);
> -	/* re-export the original imported object */
> -	if (obj->import_attach) {
> -		dmabuf = obj->import_attach->dmabuf;
> -		get_dma_buf(dmabuf);
> -		goto out_have_obj;
> -	}
> -
> -	if (obj->dma_buf) {
> -		get_dma_buf(obj->dma_buf);
> -		dmabuf = obj->dma_buf;
> -		goto out_have_obj;
> -	}
> +	} else {
> +		mutex_lock(&dev->object_name_lock);
> +		/* re-export the original imported object */
> +		if (obj->import_attach) {
> +			dmabuf = obj->import_attach->dmabuf;
> +			get_dma_buf(dmabuf);
> +		} else if (obj->dma_buf) {
> +			get_dma_buf(obj->dma_buf);
> +			dmabuf = obj->dma_buf;
> +		} else {
> +			dmabuf = export_and_register_object(dev, obj, flags);
> +			if (IS_ERR(dmabuf)) {
> +				/* normally the created dma-buf takes ownership of the ref,
> +				 * but if that fails then drop the ref
> +				 */
> +				ret = PTR_ERR(dmabuf);
> +				mutex_unlock(&dev->object_name_lock);
> +				goto out;
> +			}
> +		}
>  
> -	dmabuf = export_and_register_object(dev, obj, flags);
> -	if (IS_ERR(dmabuf)) {
> -		/* normally the created dma-buf takes ownership of the ref,
> -		 * but if that fails then drop the ref
> +		/*
> +		 * If we've exported this buffer then cheat and add it to the import list
> +		 * so we get the correct handle back. We must do this under the
> +		 * protection of dev->object_name_lock to ensure that a racing gem close
> +		 * ioctl doesn't miss to remove this buffer handle from the cache.
>  		 */
> -		ret = PTR_ERR(dmabuf);
> +		ret = drm_prime_add_buf_handle(&file_priv->prime,
> +					       dmabuf, handle);
>  		mutex_unlock(&dev->object_name_lock);
> -		goto out;
> +		if (ret) {
> +			dma_buf_put(dmabuf);
> +			goto out;
> +		}
>  	}
>  
> -out_have_obj:
> -	/*
> -	 * If we've exported this buffer then cheat and add it to the import list
> -	 * so we get the correct handle back. We must do this under the
> -	 * protection of dev->object_name_lock to ensure that a racing gem close
> -	 * ioctl doesn't miss to remove this buffer handle from the cache.
> -	 */
> -	ret = drm_prime_add_buf_handle(&file_priv->prime,
> -				       dmabuf, handle);
> -	mutex_unlock(&dev->object_name_lock);
> -	if (ret)
> -		goto fail_put_dmabuf;
> -
> -out_have_handle:
>  	ret = dma_buf_fd(dmabuf, flags);
>  	/*
>  	 * We must _not_ remove the buffer from the handle cache since the newly
> @@ -469,16 +464,12 @@ out_have_handle:
>  	 * Closing the handle will clean out the cache anyway, so we don't leak.
>  	 */
>  	if (ret < 0) {
> -		goto fail_put_dmabuf;
> +		dma_buf_put(dmabuf);
>  	} else {
>  		*prime_fd = ret;
>  		ret = 0;
>  	}
>  
> -	goto out;
> -
> -fail_put_dmabuf:
> -	dma_buf_put(dmabuf);
>  out:
>  	drm_gem_object_unreference_unlocked(obj);
>  out_unlock:
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel