[Mesa-dev] [PATCH] glsl: do not attempt to dump_shader if no shaderobj

[Iago Toral]

On Mon, 2015-09-21 at 09:15 +0300, Tapani Pälli wrote:
> Patch fixes a crash in conformance test that tries out different
> invalid arguments for glShaderSource and glGetShaderSource:
> 
>    ES2-CTS.gtf.GL.glGetShaderSource.getshadersource_programhandle
> 
> This is a regression from commit:
>    04e201d0c02cd30ace5c6fe80e9f021ebb733682
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mesa/main/shaderapi.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index f31980b..7733d02 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1699,15 +1699,17 @@ _mesa_ShaderSource(GLhandleARB shaderObj, GLsizei count,
>  #if defined(HAVE_SHA1)
>     sh = _mesa_lookup_shader(ctx, shaderObj);

Why not call this earlier in that function (before we even process the
shader string) and return if it is NULL? There is no point in waiting
until this moment to check for that.

Then, when we call shader_source right below this code, we could just
pass the sh object directly instead of having that function call
_mesa_lookup_shader again (we could maybe replace that call with an
assert to make sure that we passed a valid shader object)

Iago

> -   /* Dump original shader source to MESA_SHADER_DUMP_PATH and replace
> -    * if corresponding entry found from MESA_SHADER_READ_PATH.
> -    */
> -   dump_shader(sh->Stage, source);
> +   if (sh) {
> +      /* Dump original shader source to MESA_SHADER_DUMP_PATH and replace
> +       * if corresponding entry found from MESA_SHADER_READ_PATH.
> +       */
> +      dump_shader(sh->Stage, source);
>  
> -   replacement = read_shader(sh->Stage, source);
> -   if (replacement) {
> -      free(source);
> -      source = replacement;
> +      replacement = read_shader(sh->Stage, source);
> +      if (replacement) {
> +         free(source);
> +         source = replacement;
> +      }
>     }
>  #endif /* HAVE_SHA1 */
>