[Mesa-dev] [libdrm PATCH 1/4] libkms/intel.c: Fix a memory leak and a dead assignment as well as cleanup code a bit.

[Marcin Slusarz]

On Thu, Jun 28, 2012 at 09:51:55PM +0200, Johannes Obermayr wrote:
> ---
>  libkms/intel.c |   23 ++++++++++++-----------
>  1 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/libkms/intel.c b/libkms/intel.c
> index 8b8249b..7bf1f76 100644
> --- a/libkms/intel.c
> +++ b/libkms/intel.c
> @@ -93,14 +93,18 @@ intel_bo_create(struct kms_driver *kms,
>  	if (!bo)
>  		return -ENOMEM;
>  
> -	if (type == KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8) {
> +	switch (type) {
> +	case KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8:
>  		pitch = 64 * 4;
>  		size = 64 * 64 * 4;
> -	} else if (type == KMS_BO_TYPE_SCANOUT_X8R8G8B8) {
> +		break;
> +	case KMS_BO_TYPE_SCANOUT_X8R8G8B8:
>  		pitch = width * 4;
>  		pitch = (pitch + 512 - 1) & ~(512 - 1);
>  		size = pitch * ((height + 4 - 1) & ~(4 - 1));
> -	} else {
> +		break;
> +	default:
> +		free(bo);
>  		return -EINVAL;
>  	}
>  
> @@ -108,8 +112,10 @@ intel_bo_create(struct kms_driver *kms,
>  	arg.size = size;
>  
>  	ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_CREATE, &arg, sizeof(arg));
> -	if (ret)
> -		goto err_free;
> +	if (ret) {
> +		free(bo);
> +		return ret;
> +	}
>  
>  	bo->base.kms = kms;
>  	bo->base.handle = arg.handle;

The same comment as in nouveau patch.

> @@ -124,9 +130,8 @@ intel_bo_create(struct kms_driver *kms,
>  		tile.handle = bo->base.handle;
>  		tile.tiling_mode = I915_TILING_X;
>  		tile.stride = bo->base.pitch;
> -
> -		ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile));
>  #if 0
> +		ret = drmCommandWriteRead(kms->fd, DRM_I915_GEM_SET_TILING, &tile, sizeof(tile));
>  		if (ret) {
>  			kms_bo_destroy(out);
>  			return ret;

This is wrong. You want to ignore the return value, not remove whole ioctl call.
See commit 8838bb1d63bdb8ffa808cd41b7e0ffd2e62ff7bc.


> @@ -135,10 +140,6 @@ intel_bo_create(struct kms_driver *kms,
>  	}
>  
>  	return 0;
> -
> -err_free:
> -	free(bo);
> -	return ret;
>  }
>  
>  static int
> --