[Mesa-dev] [PATCH 1/2] i965: Validate requested GLES context version in brwCreateContext

[Kenneth Graunke]

On 11/09/2012 02:06 PM, Chad Versace wrote:
> For GLES1 and GLES2, brwCreateContext neglected to validate the requested
> context version received from the DRI layer. If DRI requested an OpenGL
> ES2 context with version 3.9, we provided it one.
>
> Before this fix, the switch statement that validated the requested GL
> context flavor was an ugly #ifdef copy-paste mess. Instead of reproducing
> the copy-past-mess for GLES1 and GLES2, I first refactored it.  Now the
> switch statement is readable.
>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>   src/mesa/drivers/dri/i965/brw_context.c | 58 ++++++++++++++-------------------
>   1 file changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 1448965..f439cf2 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -84,53 +84,45 @@ brwCreateContext(int api,
>      __DRIscreen *sPriv = driContextPriv->driScreenPriv;
>      struct intel_screen *screen = sPriv->driverPrivate;
>      struct dd_function_table functions;
> +   const unsigned req_version = major_version * 10 + minor_version;
> +   unsigned max_supported_version = 0;
>      unsigned i;
>
> -   /* Filter against the requested API and version.
> -    */
> -   switch (api) {
> -   case API_OPENGL: {
>   #ifdef TEXTURE_FLOAT_ENABLED
> -      const unsigned max_version =
> -         (screen->gen == 6 ||
> -          (screen->gen == 7 && screen->kernel_has_gen7_sol_reset))
> -         ? 30 : 21;
> +   bool has_texture_float = true;
>   #else
> -      const unsigned max_version = 21;
> +   bool has_texture_float = false;
>   #endif
> -      const unsigned req_version = major_version * 10 + minor_version;
>
> -      if (req_version > max_version) {
> -         *error = __DRI_CTX_ERROR_BAD_VERSION;
> -         return false;
> -      }
> +   bool supports_gl30 = has_texture_float &&
> +                        (screen->gen == 6 ||
> +                         (screen->gen == 7 &&
> +                          screen->kernel_has_gen7_sol_reset));
> +
> +   /* Determine max_supported_version. */
> +   switch (api) {
> +   case API_OPENGL:
> +      max_supported_version = supports_gl30 ? 30 : 21;
>         break;
> -   }
>      case API_OPENGLES:
> +      max_supported_version = 11;
> +      break;
>      case API_OPENGLES2:
> +      max_supported_version = 20;
>         break;
> -   case API_OPENGL_CORE: {
> -#ifdef TEXTURE_FLOAT_ENABLED
> -      const unsigned max_version =
> -         (screen->gen == 6 ||
> -          (screen->gen == 7 && screen->kernel_has_gen7_sol_reset))
> -         ? 31 : 0;
> -      const unsigned req_version = major_version * 10 + minor_version;
> -
> -      if (req_version > max_version) {
> -         *error = (max_version == 0)
> -            ? __DRI_CTX_ERROR_BAD_API : __DRI_CTX_ERROR_BAD_VERSION;
> -         return false;
> -      }
> +   case API_OPENGL_CORE:
> +      max_supported_version = supports_gl30 ? 31 : 0;
>         break;
> -#else
> -      *error = __DRI_CTX_ERROR_BAD_API;
> -      return false;
> -#endif
> -   }
>      default:
> +      break;
> +   }
> +
> +   if (max_supported_version == 0) {
>         *error = __DRI_CTX_ERROR_BAD_API;
>         return false;
> +   } else if (req_version > max_supported_version) {
> +      *error = __DRI_CTX_ERROR_BAD_VERSION;
> +      return false;
>      }
>
>      struct brw_context *brw = rzalloc(NULL, struct brw_context);

This is much, much nicer.  Thanks Chad.

For both patches:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>