On 16 March 2012 11:46, Chad Versace <<a href="mailto:chad.versace@linux.intel.com">chad.versace@linux.intel.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Overall this patch looks good. Below are some comments, some more important than others... <div class="im"> On 03/12/2012 02:41 PM, Paul Berry wrote: > Previous patches introduced code generation for the auto-generated > portions of the piglit-dispatch mechanism. This patch supplies the > remaining hand-written code, which includes: > > - Code to initialize piglit-dispatch. > > - The actions to be taken when looking up a function, when function > lookup fails, or when a function is unsupported. > > - Code to determine whether piglit-dispatch has been initialized, > which GL version is supported, and whether a given extension is > supported. > > - Code to look up a function using a name supplied at run-time. > > - Definitions of basic GL types. > --- > tests/util/piglit-dispatch-init.c | 122 +++++++++++++++++++ > tests/util/piglit-dispatch.c | 238 +++++++++++++++++++++++++++++++++++++ > tests/util/piglit-dispatch.h | 149 +++++++++++++++++++++++ > 3 files changed, 509 insertions(+), 0 deletions(-) > create mode 100644 tests/util/piglit-dispatch-init.c > create mode 100644 tests/util/piglit-dispatch.c > create mode 100644 tests/util/piglit-dispatch.h </div>-- snip -- <div><div class="h5"> > --- /dev/null > +++ b/tests/util/piglit-dispatch.c > @@ -0,0 +1,238 @@ > +/* Copyright 2012 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "piglit-util.h" </div></div>I think you need to #include "piglit-dispatch.h" too. </blockquote><div> Huh, I wonder why this ever worked. Oh, I see: piglit-util.h includes gl_wrap.h, which includes piglit-dispatch.h. Your suggestion is way clearer, and there's no harm in having a redundant include. I'll include piglit-dispatch.h before piglit-util.h. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > +/* Global state maintained by the Piglit dispatch mechanism: */ > + > +/** > + * Which function to call to get the address of a core function. > + */ > +static piglit_get_proc_address_function_ptr __get_core_proc_address = NULL; </div>Identifiers that begin with __ are reserved, even if the identifier has static or local scope. They need to be renamed.