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

[Paul Berry]

On 27 February 2013 10:59, 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.
>
>
Ok, with both you and Tom weighing in on this, I'm convinced.  I'll go
ahead and make the change before pushing the patches--it's an easy fix, and
there's no sense in having needless churn.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130227/e197fce4/attachment.html>