Re: [Mesa-dev] [PATCH 2/4] st/mesa: check return value of begin/end_query

[Samuel Pitoiset]

On 04/20/2016 05:43 PM, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haeh...@amd.com>

They can only indicate out of memory conditions, since the other error
conditions are caught earlier.
---
  src/mesa/state_tracker/st_cb_queryobj.c | 55 ++++++++++++++++++++-------------
  1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_queryobj.c 
b/src/mesa/state_tracker/st_cb_queryobj.c
index cdb9efc..784ffde 100644
--- a/src/mesa/state_tracker/st_cb_queryobj.c
+++ b/src/mesa/state_tracker/st_cb_queryobj.c
@@ -61,13 +61,9 @@ st_NewQueryObject(struct gl_context *ctx, GLuint id)
  }


-
  static void
-st_DeleteQuery(struct gl_context *ctx, struct gl_query_object *q)
+free_queries(struct pipe_context *pipe, struct st_query_object *stq)
  {
-   struct pipe_context *pipe = st_context(ctx)->pipe;
-   struct st_query_object *stq = st_query_object(q);
-
     if (stq->pq) {
        pipe->destroy_query(pipe, stq->pq);
        stq->pq = NULL;
@@ -77,6 +73,16 @@ st_DeleteQuery(struct gl_context *ctx, struct 
gl_query_object *q)
        pipe->destroy_query(pipe, stq->pq_begin);
        stq->pq_begin = NULL;
     }
+}
+
+
+static void
+st_DeleteQuery(struct gl_context *ctx, struct gl_query_object *q)
+{
+   struct pipe_context *pipe = st_context(ctx)->pipe;
+   struct st_query_object *stq = st_query_object(q);
+
+   free_queries(pipe, stq);

     free(stq);
  }
@@ -89,6 +95,7 @@ st_BeginQuery(struct gl_context *ctx, struct gl_query_object 
*q)
     struct pipe_context *pipe = st->pipe;
     struct st_query_object *stq = st_query_object(q);
     unsigned type;
+   bool ret = false;

     st_flush_bitmap_cache(st_context(ctx));

@@ -133,14 +140,7 @@ st_BeginQuery(struct gl_context *ctx, struct 
gl_query_object *q)

     if (stq->type != type) {
        /* free old query of different type */
-      if (stq->pq) {
-         pipe->destroy_query(pipe, stq->pq);
-         stq->pq = NULL;
-      }
-      if (stq->pq_begin) {
-         pipe->destroy_query(pipe, stq->pq_begin);
-         stq->pq_begin = NULL;
-      }
+      free_queries(pipe, stq);
        stq->type = PIPE_QUERY_TYPES; /* an invalid value */
     }

@@ -151,20 +151,25 @@ st_BeginQuery(struct gl_context *ctx, struct 
gl_query_object *q)
           stq->pq_begin = pipe->create_query(pipe, type, 0);
           stq->type = type;
        }
-      pipe->end_query(pipe, stq->pq_begin);
+      if (stq->pq_begin)
+         ret = pipe->end_query(pipe, stq->pq_begin);
     } else {
        if (!stq->pq) {
           stq->pq = pipe->create_query(pipe, type, q->Stream);
           stq->type = type;
        }
-      if (stq->pq) {
-         pipe->begin_query(pipe, stq->pq);
-      }
-      else {
-         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBeginQuery");
-         return;
-      }
+      if (stq->pq)
+         ret = pipe->begin_query(pipe, stq->pq);
     }
+
+   if (!ret) {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBeginQuery");
+
+      free_queries(pipe, stq);

Why do you need to free queries now? Was there a memleak before? And why 
not in glEndQuery too?

+      q->Active = GL_FALSE;
+      return;
+   }
+
     assert(stq->type == type);
  }

@@ -174,6 +179,7 @@ st_EndQuery(struct gl_context *ctx, struct gl_query_object 
*q)
  {
     struct pipe_context *pipe = st_context(ctx)->pipe;
     struct st_query_object *stq = st_query_object(q);
+   bool ret = false;

     st_flush_bitmap_cache(st_context(ctx));

@@ -185,7 +191,12 @@ st_EndQuery(struct gl_context *ctx, struct gl_query_object 
*q)
     }

     if (stq->pq)
-      pipe->end_query(pipe, stq->pq);
+      ret = pipe->end_query(pipe, stq->pq);
+
+   if (!ret) {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBeginQuery");

This should be "glEndQuery", right? I think it would be better to move 
the error message inside the previous branch, what do you think?

+      return;
+   }
  }



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev