[Mesa-dev] [PATCH shader-db] skip the 'GL >= x.y' line if present

[Samuel Pitoiset]

On 10/04/2016 11:18 AM, Nicolai Hähnle wrote:
> On 02.10.2016 16:27, Samuel Pitoiset wrote:
>> shaderdb runner fails at parsing shader_test files when the first
>> line inside the require block is not 'GLSL >= x.y'. This just skips
>> the GL version requirement which is actually unused and allows to
>> compile +164 shaders from piglit.
>> ---
>>  run.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/run.c b/run.c
>> index d833879..5b5afa8 100644
>> --- a/run.c
>> +++ b/run.c
>> @@ -77,6 +77,7 @@ get_shaders(const struct context_info *core, const
>> struct context_info *compat,
>>              const char *shader_name)
>>  {
>>      static const char *req = "[require]";
>> +    static const char *gl_req = "\nGL >= ";
>>      static const char *glsl_req = "\nGLSL >= ";
>>      static const char *fp_req = "\nGL_ARB_fragment_program";
>>      static const char *vp_req = "\nGL_ARB_vertex_program";
>
> FWIW, it's better to define string constants as
>
> static const char foo[] = ...;
>
> The difference is that const char * is a pointer, which means the
> compiler may emit an additional 8 bytes of constant data which also
> needs to be relocated at load time. In this particular case this may
> well get optimized away, but using the array notation instead is a good
> habit for the cases where it isn't.
>
> Yes, I know you were just following the pattern :)

Yes, consistency. :)

>
>> @@ -97,6 +98,11 @@ get_shaders(const struct context_info *core, const
>> struct context_info *compat,
>>      /* Find the [require] block and parse it first. */
>>      text = memmem(text, end_text - text, req, strlen(req)) +
>> strlen(req);
>>
>> +    /* Skip the GL >= x.y line if present. */
>> +    if (memcmp(text, gl_req, strlen(gl_req)) == 0) {
>> +        text += strlen(gl_req) + 3; /* for x.y */
>> +    }
>
> Mhh, that +3 is a bit brittle, and using memcmp when text may not
> actually be long enough is a bad idea. I suppose it gets the job done
> though and it's all just hacky test code anyway, so I'll shut up now :)

I do agree, it's a bit hacky but shaderdb runner is not robust for 
parsing shader_test files anyway. This should be improved at some point. :)

>
> Cheers,
> Nicolai
>
>> +
>>      if (memcmp(text, glsl_req, strlen(glsl_req)) == 0) {
>>          text += strlen(glsl_req);
>>          long major = strtol(text, (char **)&text, 10);
>>

-- 
-Samuel