[Piglit] [PATCH RESEND 3/3] glslparsertest: Avoid uninitialized vars in parse_glsl_version_number.

[Tom Gall]

For the series.

Reviewed-by: Tom Gall <tom.gall at linaro.org>

On Wed, Feb 27, 2013 at 12:59 PM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> On 02/27/2013 10:47 AM, Paul Berry wrote:
>> On 27 February 2013 10:44, Eric Anholt <eric at anholt.net
>> <mailto:eric at anholt.net>> wrote:
>>
>>     Paul Berry <stereotype441 at gmail.com <mailto:stereotype441 at gmail.com>> writes:
>>
>>     > Previously, if the user specified an ill-formed GLSL version number
>>     > (or the implementation supplied an ill-formed number in its response
>>     > to glGetString(GL_SHADING_LANGUAGE_VERSION)), glslparsertest would
>>     > access uninitialized variables, resulting in unpredictable (and often
>>     > confusing) behaviour.
>>     >
>>     > With this patch, glslparser test accepts version numbers either of the
>>     > form "<int>" or "<int>.<int>".  Ill-formed version numbers lead to a
>>     > test failure.
>>     > ---
>>     >  tests/glslparsertest/glslparsertest.c | 10 +++++++---
>>     >  1 file changed, 7 insertions(+), 3 deletions(-)
>>     >
>>     > diff --git a/tests/glslparsertest/glslparsertest.c
>>     b/tests/glslparsertest/glslparsertest.c
>>     > index 43bef03..c9696be 100644
>>     > --- a/tests/glslparsertest/glslparsertest.c
>>     > +++ b/tests/glslparsertest/glslparsertest.c
>>     > @@ -339,10 +339,14 @@ int process_options(int argc, char **argv)
>>     >  static unsigned
>>     >  parse_glsl_version_number(const char *str)
>>     >  {
>>     > -     unsigned major;
>>     > -     unsigned minor;
>>     > +     unsigned major = 0;
>>     > +     unsigned minor = 0;
>>     > +
>>     > +     if (sscanf(str, "%u.%u", &major, &minor) == 0) {
>>     > +             printf("Ill-formed GLSL version number: %s\n", str);
>>     > +             piglit_report_result(PIGLIT_FAIL);
>>     > +     }
>>
>>     For full pedantry, I think that would be sscanf(...) != 2.
>>
>>
>> Actually, sscanf(...) == 0 allows the version number to be supplied as either
>> "<int>" or "<int>.<int>".
>
> Please add a comment explaining that's why `== 0` is used. I saw your trick,
> but it's not obvious.
>
> With that, the series is:
> Reviewed-by: Chad Versace <chad.versace at linux.intel.com>
>
> By the way, I do prefer Tom's suggestions to use runtime detection in
> patches 1 and 2. That would be one less thing to clean up when we switch
> to unified binaries. But, it doesn't really matter today.
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit



--
Regards,
Tom

"Where's the kaboom!? There was supposed to be an earth-shattering
kaboom!" Marvin Martian
Tech Lead, Graphics Working Group | Linaro.org │ Open source software
for ARM SoCs
w) tom.gall att linaro.org
h) tom_gall att mac.com