On 27 February 2013 10:59, 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. > > 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.
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit