[Mesa-dev] [PATCH 3/5] Add more NULL checks

[Tom Stellard]

On Sat, Jan 04, 2014 at 01:27:31AM +0100, Bruno Jiménez wrote:
> In this case, NULL checks are added to compute_memory_grow_pool,
> so it returns -1 when it fails. This makes necesary
> to handle such cases in compute_memory_finalize_pending
> when it is needed to grow the pool
> ---
>  src/gallium/drivers/r600/compute_memory_pool.c | 30 ++++++++++++++++++++------
>  src/gallium/drivers/r600/compute_memory_pool.h |  6 ++++--
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/compute_memory_pool.c b/src/gallium/drivers/r600/compute_memory_pool.c
> index 62d1a5c..5374a48 100644
> --- a/src/gallium/drivers/r600/compute_memory_pool.c
> +++ b/src/gallium/drivers/r600/compute_memory_pool.c
> @@ -161,9 +161,10 @@ struct compute_memory_item* compute_memory_postalloc_chunk(
>  }
>  
>  /**
> - * Reallocates pool, conserves data
> + * Reallocates pool, conserves data.
> + * Returns -1 if it fails

s/Returns/@returns/ so doxygen can understand it, and you should also
mention the return value for success.

>   */
> -void compute_memory_grow_pool(struct compute_memory_pool* pool,
> +int compute_memory_grow_pool(struct compute_memory_pool* pool,
>  	struct pipe_context * pipe, int new_size_in_dw)
>  {
>  	COMPUTE_DBG(pool->screen, "* compute_memory_grow_pool() "
> @@ -174,6 +175,8 @@ void compute_memory_grow_pool(struct compute_memory_pool* pool,
>  
>  	if (!pool->bo) {
>  		compute_memory_pool_init(pool, MAX2(new_size_in_dw, 1024 * 16));
> +		if(pool->shadow == NULL)
> +			return -1;
>  	} else {
>  		new_size_in_dw += 1024 - (new_size_in_dw % 1024);
>  
> @@ -182,6 +185,9 @@ void compute_memory_grow_pool(struct compute_memory_pool* pool,
>  
>  		compute_memory_shadow(pool, pipe, 1);
>  		pool->shadow = realloc(pool->shadow, new_size_in_dw*4);
> +		if(pool->shadow == NULL)
> +			return -1;
> +
>  		pool->size_in_dw = new_size_in_dw;
>  		pool->screen->b.b.resource_destroy(
>  			(struct pipe_screen *)pool->screen,
> @@ -191,6 +197,8 @@ void compute_memory_grow_pool(struct compute_memory_pool* pool,
>  							pool->size_in_dw * 4);
>  		compute_memory_shadow(pool, pipe, 0);
>  	}
> +
> +	return 0;
>  }
>  
>  /**
> @@ -214,8 +222,9 @@ void compute_memory_shadow(struct compute_memory_pool* pool,
>  
>  /**
>   * Allocates pending allocations in the pool
> + * Returns -1 if it fails

s/Returns/@returns and mention success value.

>   */
> -void compute_memory_finalize_pending(struct compute_memory_pool* pool,
> +int compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  	struct pipe_context * pipe)
>  {
>  	struct compute_memory_item *pending_list = NULL, *end_p = NULL;
> @@ -226,6 +235,8 @@ void compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  
>  	int64_t start_in_dw = 0;
>  
> +	int err = 0;
> +
>  	COMPUTE_DBG(pool->screen, "* compute_memory_finalize_pending()\n");
>  
>  	for (item = pool->item_list; item; item = item->next) {
> @@ -293,7 +304,9 @@ void compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  	 * they aren't contiguous, so it will be impossible to allocate Item D.
>  	 */
>  	if (pool->size_in_dw < allocated+unallocated) {
> -		compute_memory_grow_pool(pool, pipe, allocated+unallocated);
> +		err = compute_memory_grow_pool(pool, pipe, allocated+unallocated);
> +		if(err == -1)
> +			return -1;
>  	}
>  
>  	/* Loop through all the pending items, allocate space for them and
> @@ -310,17 +323,20 @@ void compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  			need += 1024 - (need % 1024);
>  
>  			if (need > 0) {
> -				compute_memory_grow_pool(pool,
> +				err = compute_memory_grow_pool(pool,
>  						pipe,
>  						pool->size_in_dw + need);
>  			}
>  			else {
>  				need = pool->size_in_dw / 10;
>  				need += 1024 - (need % 1024);
> -				compute_memory_grow_pool(pool,
> +				err = compute_memory_grow_pool(pool,
>  						pipe,
>  						pool->size_in_dw + need);
>  			}
> +
> +			if(err == -1)
> +				return -1;
>  		}
>  		COMPUTE_DBG(pool->screen, "  + Found space for Item %p id = %u "
>  			"start_in_dw = %u (%u bytes) size_in_dw = %u (%u bytes)\n",
> @@ -356,6 +372,8 @@ void compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  
>  		allocated += item->size_in_dw;
>  	}
> +
> +	return 0;
>  }
>  
>  
> diff --git a/src/gallium/drivers/r600/compute_memory_pool.h b/src/gallium/drivers/r600/compute_memory_pool.h
> index 3777e3f..5354b9a 100644
> --- a/src/gallium/drivers/r600/compute_memory_pool.h
> +++ b/src/gallium/drivers/r600/compute_memory_pool.h
> @@ -66,8 +66,9 @@ struct compute_memory_item* compute_memory_postalloc_chunk(struct compute_memory
>  
>  /** 
>   * reallocates pool, conserves data
> + * Returns -1 if it fails
>   */

While you are cleaning this up, I think we can remove the comment blocks from the header file that are duplicated in compute_memory_pool.c

> -void compute_memory_grow_pool(struct compute_memory_pool* pool, struct pipe_context * pipe,
> +int compute_memory_grow_pool(struct compute_memory_pool* pool, struct pipe_context * pipe,
>  	int new_size_in_dw);
>  
>  /**
> @@ -78,8 +79,9 @@ void compute_memory_shadow(struct compute_memory_pool* pool,
>  
>  /**
>   * Allocates pending allocations in the pool
> + * Returns -1 if it fails
>   */

This comment block can also be removed.

> -void compute_memory_finalize_pending(struct compute_memory_pool* pool,
> +int compute_memory_finalize_pending(struct compute_memory_pool* pool,
>  	struct pipe_context * pipe);
>  void compute_memory_defrag(struct compute_memory_pool* pool); ///Defragment the memory pool, always heavy memory usage
>  void compute_memory_free(struct compute_memory_pool* pool, int64_t id);
> -- 
> 1.8.5.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev