[Mesa-dev] [PATCH 6/8] Support of RENDER_TYPE property [6/6] Verification of the RENDER_TYPE value.

Ian Romanick ian.d.romanick at intel.com
Mon Jul 15 11:55:46 PDT 2013


On 07/15/2013 07:28 AM, Tomasz Lis wrote:
> The change is to correctly handle the value of renderType in GLX context.
> In case of the value being incorrect, context creation fails.
> ---
>   src/glx/dri2_glx.c             |   11 +++++++++++
>   src/glx/dri_glx.c              |    6 ++++++
>   src/glx/drisw_glx.c            |   12 ++++++++++++
>   src/glx/glxclient.h            |    2 ++
>   src/glx/glxcmds.c              |   25 +++++++++++++++++++++++++
>   src/glx/indirect_glx.c         |    6 ++++++
>   src/mesa/drivers/x11/fakeglx.c |    5 ++++-
>   7 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index d60c675..539e7f1 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -205,6 +205,12 @@ dri2_create_context(struct glx_screen *base,
>      __GLXDRIconfigPrivate *config = (__GLXDRIconfigPrivate *) config_base;
>      __DRIcontext *shared = NULL;
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(config_base, renderType);
> +   if (!renderType) {
> +       return NULL;
> +   }
> +

Every place that uses verifyConextRenderTypeGLX should be:

     if (!verifyContextRenderTypeGLX(config_base, renderType))
         return NULL;

And see blow.

>      if (shareList) {
>         /* If the shareList context is not a DRI2 context, we cannot possibly
>          * create a DRI2 context that shares it.
> @@ -277,6 +283,11 @@ dri2_create_context_attribs(struct glx_screen *base,
>                                    error))
>         goto error_exit;
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(config_base, renderType);
> +   if (!renderType)
> +       goto error_exit;
> +
>      if (shareList) {
>         pcp_shared = (struct dri2_context *) shareList;
>         shared = pcp_shared->driContext;
> diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
> index cc45734..de44451 100644
> --- a/src/glx/dri_glx.c
> +++ b/src/glx/dri_glx.c
> @@ -581,6 +581,12 @@ dri_create_context(struct glx_screen *base,
>      if (!psc->base.driScreen)
>         return NULL;
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(config_base, renderType);
> +   if (!renderType) {
> +       return NULL;
> +   }
> +
>      if (shareList) {
>         /* If the shareList context is not a DRI context, we cannot possibly
>          * create a DRI context that shares it.
> diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
> index ef0e52b..0e45607 100644
> --- a/src/glx/drisw_glx.c
> +++ b/src/glx/drisw_glx.c
> @@ -380,6 +380,12 @@ drisw_create_context(struct glx_screen *base,
>      if (!psc->base.driScreen)
>         return NULL;
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(config_base, renderType);
> +   if (!renderType) {
> +       return NULL;
> +   }
> +
>      if (shareList) {
>         /* If the shareList context is not a DRISW context, we cannot possibly
>          * create a DRISW context that shares it.
> @@ -451,6 +457,12 @@ drisw_create_context_attribs(struct glx_screen *base,
>   				 error))
>         return NULL;
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(config_base, renderType);
> +   if (!renderType) {
> +       return NULL;
> +   }
> +
>      if (reset != __DRI_CTX_RESET_NO_NOTIFICATION)
>         return NULL;
>
> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
> index fc8f31c..0e78584 100644
> --- a/src/glx/glxclient.h
> +++ b/src/glx/glxclient.h
> @@ -803,6 +803,8 @@ extern int
>   applegl_create_display(struct glx_display *display);
>   #endif
>
> +extern int verifyContextRenderTypeGLX(const struct glx_config *config, int renderType);
> +
>
>   extern struct glx_drawable *GetGLXDrawable(Display *dpy, GLXDrawable drawable);
>   extern int InitGLXDrawable(Display *dpy, struct glx_drawable *glxDraw,
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 99b0218..e5ae7f8 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -224,6 +224,31 @@ ValidateGLXFBConfig(Display * dpy, GLXFBConfig fbconfig)
>      return NULL;
>   }
>
> +/**
> + * Verifies context's GLX_RENDER_TYPE value with config.
> + * \param config GLX FBConfig which will support the returned renderType.
> + * \param renderType The context render type to be verified.
> + * \return Gives the approved value of context renderType, or 0 if no valid value was found.
> + */

The users of this function only care that it returns 0 or non-0.  Have 
it return type Bool instead.

I also think this function could use a better name. 
validate_renderType_against_config seems more descriptive.

> +int
> +verifyContextRenderTypeGLX(const struct glx_config *config, int renderType)
> +{
> +    switch (renderType)
> +    {

Opening curly brace should be on the same line with the switch.

> +    case GLX_RGBA_TYPE:
> +        return (config->renderType & GLX_RGBA_BIT) ? GLX_RGBA_TYPE : 0;

            return (config->renderType & GLX_RGBA_BIT) != 0;

etc.

> +    case GLX_COLOR_INDEX_TYPE:
> +        return (config->renderType & GLX_COLOR_INDEX_BIT) ? GLX_COLOR_INDEX_TYPE : 0;
> +    case GLX_RGBA_FLOAT_TYPE_ARB:
> +        return (config->renderType & GLX_RGBA_FLOAT_BIT_ARB) ? GLX_RGBA_FLOAT_TYPE_ARB : 0;
> +    case GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT:
> +        return (config->renderType & GLX_RGBA_UNSIGNED_FLOAT_BIT_EXT) ? GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT : 0;
> +    default:
> +        break;
> +    }
> +    return 0;
> +}
> +
>   _X_HIDDEN Bool
>   glx_context_init(struct glx_context *gc,
>   		 struct glx_screen *psc, struct glx_config *config)
> diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
> index fc5107d..c1a81d5 100644
> --- a/src/glx/indirect_glx.c
> +++ b/src/glx/indirect_glx.c
> @@ -352,6 +352,12 @@ indirect_create_context(struct glx_screen *psc,
>         return NULL;
>      }
>
> +   /* Check the renderType value */
> +   renderType = verifyContextRenderTypeGLX(mode, renderType);
> +   if (!renderType) {
> +       return NULL;
> +   }
> +
>      /* Allocate our context record */
>      gc = calloc(1, sizeof *gc);
>      if (!gc) {
> diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c
> index 7a2cfbe..c48fc40 100644
> --- a/src/mesa/drivers/x11/fakeglx.c
> +++ b/src/mesa/drivers/x11/fakeglx.c
> @@ -2325,7 +2325,10 @@ Fake_glXCreateNewContext( Display *dpy, GLXFBConfig config,
>      XMesaVisual xmvis = (XMesaVisual) config;
>
>      if (!dpy || !config ||
> -       (renderType != GLX_RGBA_TYPE && renderType != GLX_COLOR_INDEX_TYPE))
> +       (renderType != GLX_RGBA_TYPE &&
> +          renderType != GLX_COLOR_INDEX_TYPE &&
> +       renderType != GLX_RGBA_FLOAT_TYPE_ARB &&
> +          renderType != GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT))
>         return 0;
>
>      glxCtx = CALLOC_STRUCT(fake_glx_context);
>



More information about the mesa-dev mailing list