dixSetPrivate regression

[Peter Hutterer]

On Mon, Jun 07, 2010 at 02:38:22PM -0700, Keith Packard wrote:
> On Mon, 07 Jun 2010 11:21:48 -0700, Jeremy Huddleston <jeremyhu at apple.com> wrote:
> > Yeah, XTestExtensionInit isn't getting run.  We default to having that extension disabled and allow users to enable it with a special preference (see http://cgit.freedesktop.org/xorg/xserver/tree/hw/xquartz/X11Application.m#n757 ):
> > 
> >     noTestExtensions = ![self prefs_get_boolean:@PREFS_TEST_EXTENSIONS
> >                                         default:FALSE];
> > 
> > This seems to confirm what you've already deduced.  And to add further
> > confirmation, enabling the XTest extension gets us a working server.
> 
> Here's a patch which moves the data used by XTest into the deviceintrec
> so that the code doesn't require a private at all. If this fixes the
> problem for you with XTest not enabled, and Peter likes it, I'll merge
> it in.
> 
> -keith
> 
> From 9591cceff0ebfc9590b9eedffb664fe96da4cc1b Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp at keithp.com>
> Date: Mon, 7 Jun 2010 14:23:47 -0700
> Subject: [PATCH] If XTest is always required, then eliminate the XTest devPrivate
> 
> The internals of XTest are used by Xi and Xkb, and both Xi and Xkb are
> always required, so it makes little sense to have XTest place data in
> a devPrivate, especially a devPrivate which is only available when the
> XTest extension is enabled.
> 
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>  Xext/xtest.c               |   33 ++++++---------------------------
>  include/input.h            |    1 -
>  include/inputstr.h         |    3 +++
>  test/xi2/protocol-common.c |    1 -
>  test/xtest.c               |    1 -
>  5 files changed, 9 insertions(+), 30 deletions(-)
> 
> diff --git a/Xext/xtest.c b/Xext/xtest.c
> index 6ee8430..2c733d8 100644
> --- a/Xext/xtest.c
> +++ b/Xext/xtest.c
> @@ -62,10 +62,6 @@ extern int DeviceValuator;
>   * other's memory */
>  static EventListPtr xtest_evlist;
>  
> -/* Used to store if a device is an XTest Virtual device */
> -static DevPrivateKeyRec XTestDevicePrivateKeyRec;
> -#define XTestDevicePrivateKey (&XTestDevicePrivateKeyRec)
> -
>  /**
>   * xtestpointer
>   * is the virtual pointer for XTest. It is the first slave
> @@ -99,18 +95,9 @@ static DISPATCH_PROC(SProcXTestFakeInput);
>  static DISPATCH_PROC(SProcXTestGetVersion);
>  static DISPATCH_PROC(SProcXTestGrabControl);
>  
> -Bool
> -XTestInitPrivates(void)
> -{
> -    return dixRegisterPrivateKey(&XTestDevicePrivateKeyRec, PRIVATE_DEVICE, 0);
> -}
> -
>  void
>  XTestExtensionInit(INITARGS)
>  {
> -    if (!XTestInitPrivates())
> -	return;
> -
>      AddExtension(XTestExtensionName, 0, 0,
>              ProcXTestDispatch, SProcXTestDispatch,
>              NULL, StandardMinorOpcode);
> @@ -654,8 +641,8 @@ int AllocXTestDevice (ClientPtr client, char* name,
>  
>      retval = AllocDevicePair( client, xtestname, ptr, keybd, CorePointerProc, CoreKeyboardProc, FALSE);
>      if ( retval == Success ){
> -        dixSetPrivate(&((*ptr)->devPrivates), XTestDevicePrivateKey, (void *)(intptr_t)master_ptr->id);
> -        dixSetPrivate(&((*keybd)->devPrivates), XTestDevicePrivateKey, (void *)(intptr_t)master_keybd->id);
> +	(*ptr)->xtest_master_id = master_ptr->id;
> +	(*keybd)->xtest_master_id = master_keybd->id;
>  
>          XIChangeDeviceProperty(*ptr, XIGetKnownProperty(XI_PROP_XTEST_DEVICE),
>                  XA_INTEGER, 8, PropModeReplace, 1, &dummy,
> @@ -683,23 +670,15 @@ int AllocXTestDevice (ClientPtr client, char* name,
>  BOOL
>  IsXTestDevice(DeviceIntPtr dev, DeviceIntPtr master)
>  {
> -    int is_XTest = FALSE;
> -    int mid;
> -    void *tmp; /* shut up, gcc! */
> -
>      if (IsMaster(dev))
> -        return is_XTest;
> -
> -    tmp = dixLookupPrivate(&dev->devPrivates, XTestDevicePrivateKey);
> -    mid = (intptr_t)tmp;
> +        return FALSE;
>  
>      /* deviceid 0 is reserved for XIAllDevices, non-zero mid means XTest
>       * device */
> -    if ((!master && mid) ||
> -        (master && mid == master->id))
> -        is_XTest = TRUE;
> +    if (master)
> +	return dev->xtest_master_id == master->id;
>  
> -    return is_XTest;
> +    return dev->xtest_master_id != 0;
>  }
>  
>  /**
> diff --git a/include/input.h b/include/input.h
> index 3b29855..c68a284 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -237,7 +237,6 @@ extern int key_is_down(DeviceIntPtr pDev, int key_code, int type);
>  
>  extern void InitCoreDevices(void);
>  extern void InitXTestDevices(void);
> -extern Bool XTestInitPrivates(void);
>  
>  extern _X_EXPORT DeviceIntPtr AddInputDevice(
>      ClientPtr /*client*/,
> diff --git a/include/inputstr.h b/include/inputstr.h
> index a83e863..1b504e9 100644
> --- a/include/inputstr.h
> +++ b/include/inputstr.h
> @@ -554,6 +554,9 @@ typedef struct _DeviceIntRec {
>  
>      /* coordinate transformation matrix for absolute input devices */
>      struct pixman_f_transform transform;
> +
> +    /* XTest related master device id */
> +    int xtest_master_id;
>  } DeviceIntRec;
>  
>  typedef struct {
> diff --git a/test/xi2/protocol-common.c b/test/xi2/protocol-common.c
> index ad18042..50f2b86 100644
> --- a/test/xi2/protocol-common.c
> +++ b/test/xi2/protocol-common.c
> @@ -157,7 +157,6 @@ void init_simple(void)
>      dixResetPrivates();
>      InitAtoms();
>      XkbInitPrivates();
> -    XTestInitPrivates();
>      dixRegisterPrivateKey(&XIClientPrivateKeyRec, PRIVATE_CLIENT, sizeof(XIClientRec));
>      dixRegisterPrivateKey(&miPointerScreenKeyRec, PRIVATE_SCREEN, 0);
>      dixRegisterPrivateKey(&miPointerPrivKeyRec, PRIVATE_DEVICE, 0);
> diff --git a/test/xtest.c b/test/xtest.c
> index af75f0c..6ea6862 100644
> --- a/test/xtest.c
> +++ b/test/xtest.c
> @@ -61,7 +61,6 @@ static void xtest_init_devices(void)
>      InitAtoms();
>  
>      XkbInitPrivates();
> -    XTestInitPrivates();
>  
>      /* this also inits the xtest devices */
>      InitCoreDevices();
> -- 
> 1.7.1


Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Cheers,
  Peter