<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Reviewed-by: Neha Bhende <bhenden@vmware.com> <div id="Signature"> <div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255); font-family: Calibri, Arial, Helvetica, sans-serif, "EmojiFont", "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;"> Regards, Neha </div> </div> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Brian Paul <brianp@vmware.com> Sent: Tuesday, October 9, 2018 8:06:26 AM To: mesa-dev@lists.freedesktop.org Cc: Charmaine Lee; Neha Bhende Subject: [PATCH] svga: change svga_destroy_shader_variant() to return void <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">svga_destroy_shader_variant() itself flushes and retries the command if there's a failure. So no need for the callers to do it. Other callers of the function were already ignoring the return value. This also fixes a corner-case double-free reported by Coverity (and reported by Dave Airlie). Tested with various OpenGL apps. --- src/gallium/drivers/svga/svga_pipe_fs.c | 7 +------ src/gallium/drivers/svga/svga_pipe_gs.c | 8 +------- src/gallium/drivers/svga/svga_pipe_vs.c | 7 +------ src/gallium/drivers/svga/svga_shader.c | 5 ++--- src/gallium/drivers/svga/svga_shader.h | 2 +- 5 files changed, 6 insertions(+), 23 deletions(-) diff --git a/src/gallium/drivers/svga/svga_pipe_fs.c b/src/gallium/drivers/svga/svga_pipe_fs.c index aadfb1a..52366f0 100644 --- a/src/gallium/drivers/svga/svga_pipe_fs.c +++ b/src/gallium/drivers/svga/svga_pipe_fs.c @@ -108,12 +108,7 @@ svga_delete_fs_state(struct pipe_context *pipe, void *shader) svga->state.hw_draw.fs = NULL; } - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_PS, variant); - if (ret != PIPE_OK) { - svga_context_flush(svga, NULL); - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_PS, variant); - assert(ret == PIPE_OK); - } + svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_PS, variant); } FREE((void *)fs->base.tokens); diff --git a/src/gallium/drivers/svga/svga_pipe_gs.c b/src/gallium/drivers/svga/svga_pipe_gs.c index 2fe5477..cee92a0 100644 --- a/src/gallium/drivers/svga/svga_pipe_gs.c +++ b/src/gallium/drivers/svga/svga_pipe_gs.c @@ -120,13 +120,7 @@ svga_delete_gs_state(struct pipe_context *pipe, void *shader) svga->state.hw_draw.gs = NULL; } - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_GS, variant); - if (ret != PIPE_OK) { - svga_context_flush(svga, NULL); - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_GS, - variant); - assert(ret == PIPE_OK); - } + svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_GS, variant); } FREE((void *)gs->base.tokens); diff --git a/src/gallium/drivers/svga/svga_pipe_vs.c b/src/gallium/drivers/svga/svga_pipe_vs.c index ba87cb4..3b6d2e9 100644 --- a/src/gallium/drivers/svga/svga_pipe_vs.c +++ b/src/gallium/drivers/svga/svga_pipe_vs.c @@ -199,12 +199,7 @@ svga_delete_vs_state(struct pipe_context *pipe, void *shader) svga->state.hw_draw.vs = NULL; } - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_VS, variant); - if (ret != PIPE_OK) { - svga_context_flush(svga, NULL); - ret = svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_VS, variant); - assert(ret == PIPE_OK); - } + svga_destroy_shader_variant(svga, SVGA3D_SHADERTYPE_VS, variant); } FREE((void *)vs->base.tokens); diff --git a/src/gallium/drivers/svga/svga_shader.c b/src/gallium/drivers/svga/svga_shader.c index 7a0bb3d..22e4498 100644 --- a/src/gallium/drivers/svga/svga_shader.c +++ b/src/gallium/drivers/svga/svga_shader.c @@ -541,7 +541,7 @@ svga_new_shader_variant(struct svga_context *svga) } -enum pipe_error +void svga_destroy_shader_variant(struct svga_context *svga, SVGA3dShaderType type, struct svga_shader_variant *variant) @@ -557,6 +557,7 @@ svga_destroy_shader_variant(struct svga_context *svga, /* flush and try again */ svga_context_flush(svga, NULL); ret = SVGA3D_vgpu10_DestroyShader(svga->swc, variant->id); + assert(ret == PIPE_OK); } util_bitmask_clear(svga->shader_id_bm, variant->id); } @@ -583,8 +584,6 @@ svga_destroy_shader_variant(struct svga_context *svga, FREE(variant); svga->hud.num_shaders--; - - return ret; } /* diff --git a/src/gallium/drivers/svga/svga_shader.h b/src/gallium/drivers/svga/svga_shader.h index b80cf18..68991e7 100644 --- a/src/gallium/drivers/svga/svga_shader.h +++ b/src/gallium/drivers/svga/svga_shader.h @@ -285,7 +285,7 @@ svga_set_shader(struct svga_context *svga, struct svga_shader_variant * svga_new_shader_variant(struct svga_context *svga); -enum pipe_error +void svga_destroy_shader_variant(struct svga_context *svga, SVGA3dShaderType type, struct svga_shader_variant *variant); -- 2.7.4 </div> </div> </body> </html>