On 02/27/2013 10:47 AM, Paul Berry wrote:
> On 27 February 2013 10:44, Eric Anholt <e...@anholt.net
> <mailto:e...@anholt.net>> wrote:
> 
>     Paul Berry <stereotype...@gmail.com <mailto:stereotype...@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.vers...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to