[PATCH 1/5] Replace DDXBEFORERESET with a more general way of doing DDX-specific hooks

Wed Apr 7 10:32:02 PDT 2010

I'm confused about whether multiple declarations of the same global
are allowed. In this case, ddxHooks is declared in both
xwin/InitOutput.c and dispatch.c. But as far as I can tell, this can't
hurt any DDX except Xwin, and I assume you've tested that it works
there, so I'd guess it's fine. :-)

Reviewed-by: Jamey Sharp <jamey at minilop.net>

On Wed, Apr 7, 2010 at 9:18 AM, Jon TURNEY <jon.turney at dronecode.org.uk> wrote:
> If we use DDXBEFORERESET to control if the DIX calls ddxBeforeReset(),
> all DDX built at the same time must provide that function, whether they
> need it or not.
>
> Instead use (a structure of) function pointers, which can be initialized
> as required by the specific DDX
>
> Signed-off-by: Jon TURNEY <jon.turney at dronecode.org.uk>
> ---
>  configure.ac            |    1 -
>  dix/dispatch.c          |    7 ++++---
>  hw/dmx/dmxinit.c        |    6 ------
>  hw/vfb/InitOutput.c     |    7 -------
>  hw/xnest/Init.c         |    6 ------
>  hw/xwin/InitOutput.c    |    9 ++++++---
>  include/ddxhooks.h      |   37 +++++++++++++++++++++++++++++++++++++
>  include/dix-config.h.in |    3 ---
>  include/os.h            |    4 ----
>  9 files changed, 47 insertions(+), 33 deletions(-)
>  create mode 100644 include/ddxhooks.h
>
> diff --git a/configure.ac b/configure.ac
> index 591d2b4..73e582b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1873,7 +1873,6 @@ if test "x$XWIN" = xyes; then
>        fi
>
>        AC_DEFINE(DDXOSVERRORF, 1, [Use OsVendorVErrorF])
> -       AC_DEFINE(DDXBEFORERESET, 1, [Use ddxBeforeReset ])
>        if test "x$XF86VIDMODE" = xyes; then
>                AC_MSG_NOTICE([Disabling XF86VidMode extension])
>                XF86VIDMODE=no
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 982c808..dd16c2f 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -130,6 +130,7 @@ int ProcInitialConnection();
>  #include "inputstr.h"
>  #include "xkbsrv.h"
>  #include "site.h"
> +#include "ddxhooks.h"
>
>  #ifdef XSERVER_DTRACE
>  #include "registry.h"
> @@ -462,9 +463,9 @@ Dispatch(void)
>        }
>        dispatchException &= ~DE_PRIORITYCHANGE;
>     }
> -#if defined(DDXBEFORERESET)
> -    ddxBeforeReset ();
> -#endif
> +
> +    if (ddxHooks.ddxBeforeReset) ddxHooks.ddxBeforeReset();
> +
>     KillAllClients();
>     xfree(clientReady);
>     dispatchException &= ~DE_RESET;
> diff --git a/hw/dmx/dmxinit.c b/hw/dmx/dmxinit.c
> index f481cf5..e5598e3 100644
> --- a/hw/dmx/dmxinit.c
> +++ b/hw/dmx/dmxinit.c
> @@ -846,12 +846,6 @@ void AbortDDX(void)
>     }
>  }
>
> -#ifdef DDXBEFORERESET
> -void ddxBeforeReset(void)
> -{
> -}
> -#endif
> -
>  /** This function is called in Xserver/dix/main.c from \a main() when
>  * dispatchException & DE_TERMINATE (which is the only way to exit the
>  * main loop without an interruption. */
> diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
> index e7dd1d9..a847122 100644
> --- a/hw/vfb/InitOutput.c
> +++ b/hw/vfb/InitOutput.c
> @@ -236,13 +236,6 @@ OsVendorFatalError(void)
>  {
>  }
>
> -#if defined(DDXBEFORERESET)
> -void ddxBeforeReset(void)
> -{
> -    return;
> -}
> -#endif
> -
>  void
>  ddxUseMsg(void)
>  {
> diff --git a/hw/xnest/Init.c b/hw/xnest/Init.c
> index 8a90cc6..73b9fa2 100644
> --- a/hw/xnest/Init.c
> +++ b/hw/xnest/Init.c
> @@ -146,9 +146,3 @@ void OsVendorFatalError(void)
>     return;
>  }
>
> -#if defined(DDXBEFORERESET)
> -void ddxBeforeReset(void)
> -{
> -    return;
> -}
> -#endif
> diff --git a/hw/xwin/InitOutput.c b/hw/xwin/InitOutput.c
> index 175cd9d..2ecca37 100644
> --- a/hw/xwin/InitOutput.c
> +++ b/hw/xwin/InitOutput.c
> @@ -57,6 +57,7 @@ typedef HRESULT (*SHGETFOLDERPATHPROC)(
>     LPTSTR pszPath
>  );
>  #endif
> +#include "ddxhooks.h"
>
>
>  /*
> @@ -190,13 +191,12 @@ winClipboardShutdown (void)
>  #endif
>
>
> -#if defined(DDXBEFORERESET)
>  /*
>  * Called right before KillAllClients when the server is going to reset,
>  * allows us to shutdown our seperate threads cleanly.
>  */
>
> -void
> +static void
>  ddxBeforeReset (void)
>  {
>   winDebug ("ddxBeforeReset - Hello\n");
> @@ -205,8 +205,11 @@ ddxBeforeReset (void)
>   winClipboardShutdown ();
>  #endif
>  }
> -#endif
>
> +DdxHooks ddxHooks =
> +  {
> +    .ddxBeforeReset = ddxBeforeReset
> +  };
>
>  /* See Porting Layer Definition - p. 57 */
>  void
> diff --git a/include/ddxhooks.h b/include/ddxhooks.h
> new file mode 100644
> index 0000000..4d69508
> --- /dev/null
> +++ b/include/ddxhooks.h
> @@ -0,0 +1,37 @@
> +/*
> +  Copyright © 2009 Jon TURNEY
> +
> +  Permission is hereby granted, free of charge, to any person obtaining a
> +  copy of this software and associated documentation files (the "Software"),
> +  to deal in the Software without restriction, including without limitation
> +  the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +  and/or sell copies of the Software, and to permit persons to whom the
> +  Software is furnished to do so, subject to the following conditions:
> +
> +  The above copyright notice and this permission notice (including the next
> +  paragraph) shall be included in all copies or substantial portions of the
> +  Software.
> +
> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +  THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +  DEALINGS IN THE SOFTWARE.
> +*/
> +
> +#ifndef DDXHOOKS_H
> +#define DDXHOOKS_H
> +
> +struct _DdxHooks
> +{
> +  void (*ddxBeforeReset)(void);
> +};
> +
> +typedef struct _DdxHooks DdxHooks;
> +
> +/* zero-initialized tentative definition of ddxHooks, which may be overriden by DDX */
> +DdxHooks ddxHooks;
> +
> +#endif /* DDXHOOKS_H */
> diff --git a/include/dix-config.h.in b/include/dix-config.h.in
> index 058c8fd..07e9593 100644
> --- a/include/dix-config.h.in
> +++ b/include/dix-config.h.in
> @@ -36,9 +36,6 @@
>  /* Use OsVendorVErrorF */
>  #undef DDXOSVERRORF
>
> -/* Use ddxBeforeReset */
> -#undef DDXBEFORERESET
> -
>  /* Build DPMS extension */
>  #undef DPMSExtension
>
> diff --git a/include/os.h b/include/os.h
> index 453ab82..9f0a284 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -83,10 +83,6 @@ typedef struct _NewClientRec *NewClientPtr;
>  #include <stdio.h>
>  #include <stdarg.h>
>
> -#ifdef DDXBEFORERESET
> -extern void ddxBeforeReset (void);
> -#endif
> -
>  #ifdef DDXOSVERRORF
>  extern _X_EXPORT void (*OsVendorVErrorFProc)(const char *, va_list args);
>  #endif
> --
> 1.7.0.4