Re: [Mesa-dev] [PATCH 3/5] Add more NULL checks
On Mon, Jan 6, 2014 at 10:15 AM, 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
Same comment as 2/5. Please add spaces between if and parens.
Alex
> ---
> 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..4f4bad2 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, 0 otherwise
> */
> -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, 0 otherwise
> */
> -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
[Mesa-dev] [PATCH 3/5] Add more NULL checks
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..4f4bad2 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, 0 otherwise
*/
-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, 0 otherwise
*/
-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..e61c003 100644
--- a/src/gallium/drivers/r600/com
Re: [Mesa-dev] [PATCH 3/5] Add more NULL checks
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 @@ vo
[Mesa-dev] [PATCH 3/5] Add more NULL checks
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
*/
-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
*/
-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
