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

[Tapani Pälli]

On 09/21/2015 05:04 PM, Iago Toral wrote:
> 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)

True, it can be simplified. I did not want to touch original code 
because it works but while being here I can give it a shot.

> 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 */
>>
>
>