[Mesa-dev] [PATCH 05/16] glx/dri2: rework __DRIextension handling

[Kristian Høgsberg]

On Sun, Mar 16, 2014 at 6:48 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Use a const arrays with the extensions, and use the correct one at
> runtime. This removes the assumption that the dri2_screen has a
> fixed size dri extension list and avoids the individual assignment
> of each extension at runtime.

This change seems like unnecessary churn.  Your new mechanism doesn't
scale to multiple optional extensions, where it turns into a
combinatorial explosion.  The array size is of course correlated with
the max number of extensions you might add to the array and we could
add an assert to make sure we don't overwrite it.

> While we're here make sure that the DRI*Extensions report the
> version of the interface implemented over the listed in the headers.
> While both are currently the same, this may change in the future.

This part is good.

> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  src/glx/dri2_glx.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index acd8427..84ae875 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -74,7 +74,7 @@ struct dri2_display
>
>     __glxHashTable *dri2Hash;
>
> -   const __DRIextension *loader_extensions[4];
> +   const __DRIextension **loader_extensions;
>  };
>
>  struct dri2_context
> @@ -968,7 +968,21 @@ static const __DRIdri2LoaderExtension dri2LoaderExtension_old = {
>  };
>
>  static const __DRIuseInvalidateExtension dri2UseInvalidate = {
> -   { __DRI_USE_INVALIDATE, __DRI_USE_INVALIDATE_VERSION }
> +   .base = { __DRI_USE_INVALIDATE, 1 }
> +};
> +
> +static const __DRIextension *loader_extensions_old[] = {
> +   &dri2LoaderExtension_old.base,
> +   &systemTimeExtension.base,
> +   &dri2UseInvalidate.base,
> +   NULL,
> +};
> +
> +static const __DRIextension *loader_extensions[] = {
> +   &dri2LoaderExtension.base,
> +   &systemTimeExtension.base,
> +   &dri2UseInvalidate.base,
> +   NULL,
>  };
>
>  _X_HIDDEN void
> @@ -1240,15 +1254,13 @@ dri2CreateScreen(int screen, struct glx_display * priv)
>     if (psc->dri2->base.version >= 4) {
>        psc->driScreen =
>           psc->dri2->createNewScreen2(screen, psc->fd,
> -                                     (const __DRIextension **)
> -                                     &pdp->loader_extensions[0],
> +                                     pdp->loader_extensions,
>                                       extensions,
>                                       &driver_configs, psc);
>     } else {
>        psc->driScreen =
>           psc->dri2->createNewScreen(screen, psc->fd,
> -                                    (const __DRIextension **)
> -                                    &pdp->loader_extensions[0],
> +                                    pdp->loader_extensions,
>                                      &driver_configs, psc);
>     }
>
> @@ -1365,7 +1377,7 @@ _X_HIDDEN __GLXDRIdisplay *
>  dri2CreateDisplay(Display * dpy)
>  {
>     struct dri2_display *pdp;
> -   int eventBase, errorBase, i;
> +   int eventBase, errorBase;
>
>     if (!DRI2QueryExtension(dpy, &eventBase, &errorBase))
>        return NULL;
> @@ -1386,17 +1398,10 @@ dri2CreateDisplay(Display * dpy)
>     pdp->base.destroyDisplay = dri2DestroyDisplay;
>     pdp->base.createScreen = dri2CreateScreen;
>
> -   i = 0;
>     if (pdp->driMinor < 1)
> -      pdp->loader_extensions[i++] = &dri2LoaderExtension_old.base;
> +      pdp->loader_extensions = loader_extensions_old;
>     else
> -      pdp->loader_extensions[i++] = &dri2LoaderExtension.base;
> -
> -   pdp->loader_extensions[i++] = &systemTimeExtension.base;
> -
> -   pdp->loader_extensions[i++] = &dri2UseInvalidate.base;
> -
> -   pdp->loader_extensions[i++] = NULL;
> +      pdp->loader_extensions = loader_extensions;
>
>     pdp->dri2Hash = __glxHashCreate();
>     if (pdp->dri2Hash == NULL) {
> --
> 1.9.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev