[Mesa-stable] [Review Request (main branch)] svga: check return value from svga_set_shader( SVGA3D_SHADERTYPE_GS, NULL)

[Jose Fonseca]

Great find indeed!

It's one of those things that looks fine, even when one stares at it.

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

BTW, GCC/Clang has `__attribute__((warn_unused_result))`.  Maybe we 
could start annotate svga_set_shader() and similar functions with it.

MSVC has something similar but requires compiling with static code 
analysis to work (https://stackoverflow.com/a/22759336.)  At any rate, 
catching this sort of issue with GCC would cover Windows driver too

Jose

On 22/06/17 20:04, Charmaine Lee wrote:
> 
> Great find!
> 
> Reviewed-by: Charmaine Lee <charmainel at vmware.com>
> 
> ________________________________________
> From: Brian Paul <brianp at vmware.com>
> Sent: Thursday, June 22, 2017 12:01 PM
> To: mks-hackers at vmware.com
> Cc: Charmaine Lee; Neha Bhende; Brian Paul; mesa-stable at lists.freedesktop.org
> Subject: [Review Request (main branch)] svga: check return value from svga_set_shader( SVGA3D_SHADERTYPE_GS, NULL)
> 
> If the call fails we need to flush the command buffer and retry.  In this
> case, we were failing to unbind the GS which led to subsequent errors.
> 
> This fixes a bug replaying a Cinebench R15 apitrace in a Linux guest.
> VMware bug 1894451
> 
> cc: mesa-stable at lists.freedesktop.org
> ---
>   src/gallium/drivers/svga/svga_state_gs.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/src/gallium/drivers/svga/svga_state_gs.c b/src/gallium/drivers/svga/svga_state_gs.c
> index cff11ad..2174638 100644
> --- a/src/gallium/drivers/svga/svga_state_gs.c
> +++ b/src/gallium/drivers/svga/svga_state_gs.c
> @@ -190,6 +190,8 @@ emit_hw_gs(struct svga_context *svga, unsigned dirty)
>             *  Needs to unbind the geometry shader.
>             */
>            ret = svga_set_shader(svga, SVGA3D_SHADERTYPE_GS, NULL);
> +         if (ret != PIPE_OK)
> +            goto done;
>            svga->state.hw_draw.gs = NULL;
>         }
>         goto done;
> --
> 1.9.1
>