[PATCH] Don't abort if swrast library is not present

[Kristian Høgsberg]

On Thu, Jul 24, 2008 at 2:49 PM, Daniel Drake <dsd at laptop.org> wrote:
> GLX is enabled by default, but the current swrast behaviour causes X
> to abort with fatal error if the swrast dri library dlopen fails.
>
> Handle the case where the swrast library is not present, and do not
> register the GLX extension unless at least one screen has a usable
> GL provider.
>
> ---
>  glx/glxdriswrast.c |    6 ++++++
>  glx/glxext.c       |   37 ++++++++++++++++++++++++-------------
>  2 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
> index cf4827e..20c39ab 100644
> --- a/glx/glxdriswrast.c
> +++ b/glx/glxdriswrast.c
> @@ -468,6 +468,12 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
>
>     screen->driver = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
>     if (screen->driver == NULL) {
> +       if (errno == ENOENT) {
> +           xfree(screen);
> +           LogMessage(X_INFO, "AIGLX: swrast library %s not found\n",
> +                      filename);
> +           return NULL;
> +       }
>        LogMessage(X_ERROR, "AIGLX error: dlopen of %s failed (%s)\n",
>                   filename, dlerror());
>         goto handle_error;

This bit isn't necessary - we want to handle all other failure cases
(missing symbols in the driver, permission problems etc) gracefully
too, so I'd just change the FatalError in the handle_error path to a
LogMessage(X_ERROR, ...).

> diff --git a/glx/glxext.c b/glx/glxext.c
> index 13c65da..6ba404f 100644
> --- a/glx/glxext.c
> +++ b/glx/glxext.c
> @@ -279,6 +279,7 @@ void GlxExtensionInit(void)
>     ScreenPtr pScreen;
>     int i;
>     __GLXprovider *p;
> +    Bool glx_provided = False;
>
>     __glXContextRes = CreateNewResourceType((DeleteType)ContextGone);
>     __glXDrawableRes = CreateNewResourceType((DeleteType)DrawableGone);
> @@ -289,6 +290,29 @@ void GlxExtensionInit(void)
>     if (!AddCallback (&ClientStateCallback, glxClientCallback, 0))
>        return;
>
> +    for (i = 0; i < screenInfo.numScreens; i++) {
> +       pScreen = screenInfo.screens[i];
> +
> +       for (p = __glXProviderStack; p != NULL; p = p->next) {
> +           if (p->screenProbe(pScreen) != NULL) {
> +               LogMessage(X_INFO,
> +                          "GLX: Initialized %s GL provider for screen %d\n",
> +                          p->name, i);
> +               break;
> +           }
> +       }
> +
> +       if (!p)
> +           LogMessage(X_INFO,
> +                      "GLX: no usable GL providers found for screen %d\n", i);
> +       else
> +           glx_provided = True;
> +    }
> +
> +    /* don't register extension if GL is not provided on any screen */
> +    if (!glx_provided)
> +       return;
> +

It gets a little icky here - should we clean up the screens that
potentially initialized correctly in case glx initialization fails for
one of the screens?  I think it's okay to just leave the initialized
screens around and then let them clean up at CloseScreen time, so your
patch looks fine.

cheers,
Kristian