On 03.04.2017 20:53, Bartosz Tomczyk wrote:
Actually, I can just set only batch->used to 0, but it seems to error prone. When someone adds some fields to batch struct, it will be easy to miss that it should be initialized in glthread_unmarshal_batch.
Better to have it fail early and loudly with garbage data, rather than silently if 0 happens to be an accepted value. I think it should be changed (unless there's a good reason to rely on it on purpose, but I don't see one...).
Cheers, Nicolai
Anyway I can change it if you want to. On Mon, Apr 3, 2017 at 8:42 PM, Nicolai Hähnle <nhaeh...@gmail.com <mailto:nhaeh...@gmail.com>> wrote: On 03.04.2017 20:38, Bartosz Tomczyk wrote: --- src/mesa/main/glthread.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c index 3f07c420d4..aa14292e59 100644 --- a/src/mesa/main/glthread.c +++ b/src/mesa/main/glthread.c @@ -53,7 +53,8 @@ glthread_allocate_batch(struct gl_context *ctx) } static void -glthread_unmarshal_batch(struct gl_context *ctx, struct glthread_batch *batch) +glthread_unmarshal_batch(struct gl_context *ctx, struct glthread_batch *batch, + const bool release_batch) { size_t pos = 0; @@ -64,7 +65,10 @@ glthread_unmarshal_batch(struct gl_context *ctx, struct glthread_batch *batch) assert(pos == batch->used); - free(batch); + if (release_batch) + free(batch); + else + memset(batch, 0, offsetof(struct glthread_batch, buffer)); Hmm. Why do we memset the batch? Seems like a trivial optimization to just not do that... Apart from that, the patch looks good to me. Cheers, Nicolai } static void * @@ -103,7 +107,7 @@ glthread_worker(void *data) glthread->busy = true; pthread_mutex_unlock(&glthread->mutex); - glthread_unmarshal_batch(ctx, batch); + glthread_unmarshal_batch(ctx, batch, true); pthread_mutex_lock(&glthread->mutex); glthread->busy = false; @@ -214,7 +218,7 @@ _mesa_glthread_flush_batch_locked(struct gl_context *ctx) * need to restore it when it returns. */ if (false) { - glthread_unmarshal_batch(ctx, batch); + glthread_unmarshal_batch(ctx, batch, true); _glapi_set_dispatch(ctx->CurrentClientDispatch); return; } @@ -269,9 +273,8 @@ _mesa_glthread_finish(struct gl_context *ctx) if (!(glthread->batch_queue || glthread->busy)) { if (glthread->batch && glthread->batch->used) { struct _glapi_table *dispatch = _glapi_get_dispatch(); - glthread_unmarshal_batch(ctx, glthread->batch); + glthread_unmarshal_batch(ctx, glthread->batch, false); _glapi_set_dispatch(dispatch); - glthread_allocate_batch(ctx); } } else { _mesa_glthread_flush_batch_locked(ctx); -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte.
-- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
