For the series. Reviewed-by: Tom Gall <tom.g...@linaro.org>
On Wed, Feb 27, 2013 at 12:59 PM, Chad Versace <chad.vers...@linux.intel.com> wrote: > 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 -- 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 _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit