[Piglit] [PATCH 09/12] glx_arb_create_context: Verify rejection of forward-compatible flag w/pre-3.0

Chad Versace chad.versace at linux.intel.com
Tue Dec 20 15:23:08 PST 2011


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/14/2011 10:47 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> NVIDIA's closed-source driver fails this test because it generates the
> wrong X error (BadAlloc instead of BadMatch).  It correctly does not
> create the context.
> 
> AMD's closed-source driver fails this test because it does not
> generate any X error.  It correctly does not create the context.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  tests/all.tests                                    |    1 +
>  .../spec/glx_arb_create_context/CMakeLists.gl.txt  |    1 +
>  .../invalid-flag-forward-compatible.c              |  143 ++++++++++++++++++++
>  3 files changed, 145 insertions(+), 0 deletions(-)
>  create mode 100644 tests/spec/glx_arb_create_context/invalid-flag-forward-compatible.c
> 



> +static bool try_version(int major, int minor)

Please document try_version(). Try to do what with version? Return true
if context creation succeeds or fails?

It's obvious once you read the function body. But a comment would have taken 2
seconds read, rather than 30 for the whole function.



> +	static const struct {
> +		int major;
> +		int minor;
> +		bool must_pass;
> +	} all_gl_versions[] = {
> +		{ 1, 0, true },
> +		{ 1, 1, true },
> +		{ 1, 2, true },
> +		{ 1, 3, false },
> +		{ 1, 4, false },
> +		{ 1, 5, false },
> +		{ 2, 0, false },
> +		{ 2, 1, false }

This array is immensely confusing for two reasons, both of which could be solved by comments.

1. The name 'must_pass' is misleading, and could be interpreted two ways: that context creation
   must pass (succeed), or that try_version() must pass. Unfortunately, I assumed it was the former,
   which led to much head scratching. Please document that 'must_pass' means the latter.

2. Why is must_pass set only for versions 1.0 - 1.2? Do we not care about the other versions?
   I know the answer, but I don't expect the next person to encounter this code to.

- ----
Chad Versace
chad.versace at linux.intel.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJO8RjbAAoJEAIvNt057x8i0/cP/RwBNebVcGwLPbA34KTEOk9U
ANZbkDG8NV45zaPRF06vpxATjlztzA/ZBEWUEjzQVyIKZXhNILb4TBsu4FR7FQ1/
AgRR0VcxBbOX9AapO2bu3QZ1cFO/G9EInV3vcBUn1+SvtUdUpYZGIUQJf1hMx1xE
AEpuuNCE3/mJB1iKVYoYdHetTN6BjUf1WmUaBtArFhtglGTE/FoUXR0Nd4rlawqH
Vs/TEo+3ZjZTVvdPYW6yQUFR79yXSpArH2mJENOuLTdhejQmYTiy0K9la5um3uN4
hwxDvGYq557iLF30RsHC+X4jAITjalkQYbm1Cjjj6Py1eMkDOmdZuu16ugTNsyi9
xuqOW5XvL3KJagJaou1e8/iCbmFWMGSp2iurBQhbgp9ALvmowmoQI1uF9BD9efFd
yVfERj0UiWLjc6ftKCkFgHBoLuAG/g76jN9nkqBtlaXIKf9nQXIuuARrK78eVXMt
eaXHujxTpYowHA9Ap3fnptZK1PBR6JapchHQYeIqAVXY997wJkYEiGH3XeClpOlC
3YprZgLdNXz3aMvLpWi0LZjToogZlas/BqW6YQABtw7gxKz6ksfiBO6hgps5vXGJ
RMZ8m9oGCzGg2P2yHnyXSAmO3vQetyHl4Hy+FyOSyt6lIJhhAnI/z8/D54qIdZj7
siGedYQ6xvPcMthUTIJP
=/YyV
-----END PGP SIGNATURE-----


More information about the Piglit mailing list