[PATCH v2 libXi 1/2] SizeClassInfo can return 0 even without an error

2016-10-12 Thread Peter Hutterer
From: Niels Ole Salscheider 

Catch the error case separately. Commit 19a9cd607d added length checking to
SizeClassInfo but re-used the return value of 0 for an error. A device without
classes (as is initialized by xf86-input-libinput for tablets) can
legitimately return 0 and erroneously triggers an error.
Fix this by using a separate value for the error.

Reproducible by calling XListInputDevices() with a tablet attached.

This fixes a regression introduced in commit 19a9cd607d.

Signed-off-by: Niels Ole Salscheider 
Signed-off-by: Peter Hutterer 
---
Changes to v1:
- don't touch *size until we're sure.
- expand commit message

Niels:
I left you as author and your signed-off-by since it's essentially your
patch with a minor change.

 src/XListDev.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/XListDev.c b/src/XListDev.c
index f850cd0..e4bd3d5 100644
--- a/src/XListDev.c
+++ b/src/XListDev.c
@@ -73,27 +73,28 @@ static int pad_to_xid(int base_size)
 return ((base_size + padsize - 1)/padsize) * padsize;
 }
 
-static size_t
-SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
+static int
+SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size)
 {
-int size = 0;
 int j;
+size_t sz = 0;
+
 for (j = 0; j < num_classes; j++) {
 switch ((*any)->class) {
 case KeyClass:
-size += pad_to_xid(sizeof(XKeyInfo));
+sz += pad_to_xid(sizeof(XKeyInfo));
 break;
 case ButtonClass:
-size += pad_to_xid(sizeof(XButtonInfo));
+sz += pad_to_xid(sizeof(XButtonInfo));
 break;
 case ValuatorClass:
 {
 xValuatorInfoPtr v;
 
 if (len < sizeof(v))
-return 0;
+return 1;
 v = (xValuatorInfoPtr) *any;
-size += pad_to_xid(sizeof(XValuatorInfo) +
+sz += pad_to_xid(sizeof(XValuatorInfo) +
 (v->num_axes * sizeof(XAxisInfo)));
 break;
 }
@@ -101,11 +102,13 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int 
num_classes)
 break;
 }
 if ((*any)->length > len)
-return 0;
+return 1;
 *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length);
 }
 
-return size;
+*size = sz;
+
+return 0;
 }
 
 static void
@@ -220,8 +223,7 @@ XListInputDevices(
sav_any = any;
end = (char *)list + rlen;
for (i = 0; i < *ndevices; i++, list++) {
-s = SizeClassInfo(, end - (char *)any, (int)list->num_classes);
-if (!s)
+if(SizeClassInfo(, end - (char *)any, (int)list->num_classes, 
))
 goto out;
 size += s;
}
-- 
2.7.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH v2 libXi 2/2] XListInputDevices: don't touch ndevices in case of error

2016-10-12 Thread Peter Hutterer
We used to always set *ndevices to the number of devices returned by the
server. This magically worked because we pretty much never returned an error
except on faulty server or library implementations. With 19a9cd60 we now have
more chances of getting an error, so the polite thing is to just leave *ndevices
alone when we error out.

Document it as such in the man page, just in case someone accidentally reads
it.

Signed-off-by: Peter Hutterer 
CC: Niels Ole Salscheider 
---
Changes to v1:
- Niels' first patch set ndevices to 0, this one leaves it untouched

 man/XListInputDevices.txt | 12 ++--
 src/XListDev.c| 21 -
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/man/XListInputDevices.txt b/man/XListInputDevices.txt
index 276660d..450f377 100644
--- a/man/XListInputDevices.txt
+++ b/man/XListInputDevices.txt
@@ -220,5 +220,13 @@ DESCRIPTION
Floating. If the device is a master device, attached specifies
the device ID of the master device this device is paired with.
 
-   To free the XDeviceInfo array created by XListInputDevices, use
-   XFreeDeviceList.
+RETURN VALUE
+
+
+   XListInputDevices returns a pointer to an array of XDeviceInfo
+   structs and sets ndevices_return to the number of elements in
+   that array. To free the XDeviceInfo array created by
+   XListInputDevices, use XFreeDeviceList.
+
+   On error, XListInputDevices returns NULL and ndevices_return is
+   left unmodified.
diff --git a/src/XListDev.c b/src/XListDev.c
index e4bd3d5..dda6011 100644
--- a/src/XListDev.c
+++ b/src/XListDev.c
@@ -175,7 +175,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int 
num_classes)
 XDeviceInfo *
 XListInputDevices(
 register Display   *dpy,
-int*ndevices)
+int*ndevices_return)
 {
 size_t s, size;
 xListInputDevicesReq *req;
@@ -190,6 +190,7 @@ XListInputDevices(
 int i;
 unsigned long rlen;
 XExtDisplayInfo *info = XInput_find_display(dpy);
+int ndevices;
 
 LockDisplay(dpy);
 if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
@@ -205,8 +206,8 @@ XListInputDevices(
return (XDeviceInfo *) NULL;
 }
 
-if ((*ndevices = rep.ndevices)) {  /* at least 1 input device */
-   size = *ndevices * sizeof(XDeviceInfo);
+if ((ndevices = rep.ndevices)) {   /* at least 1 input device */
+   size = ndevices * sizeof(XDeviceInfo);
if (rep.length < (INT_MAX >> 2)) {
rlen = rep.length << 2; /* multiply length by 4*/
slist = list = Xmalloc(rlen);
@@ -219,17 +220,17 @@ XListInputDevices(
}
_XRead(dpy, (char *)list, rlen);
 
-   any = (xAnyClassPtr) ((char *)list + (*ndevices * sizeof(xDeviceInfo)));
+   any = (xAnyClassPtr) ((char *)list + (ndevices * sizeof(xDeviceInfo)));
sav_any = any;
end = (char *)list + rlen;
-   for (i = 0; i < *ndevices; i++, list++) {
+   for (i = 0; i < ndevices; i++, list++) {
 if(SizeClassInfo(, end - (char *)any, (int)list->num_classes, 
))
 goto out;
 size += s;
}
 
Nptr = ((unsigned char *)list) + rlen;
-   for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) {
+   for (i = 0, nptr = (unsigned char *)any; i < ndevices; i++) {
if (nptr >= Nptr)
goto out;
size += *nptr + 1;
@@ -245,10 +246,10 @@ XListInputDevices(
}
sclist = clist;
Any = (XAnyClassPtr) ((char *)clist +
- (*ndevices * sizeof(XDeviceInfo)));
+ (ndevices * sizeof(XDeviceInfo)));
list = slist;
any = sav_any;
-   for (i = 0; i < *ndevices; i++, list++, clist++) {
+   for (i = 0; i < ndevices; i++, list++, clist++) {
clist->type = list->type;
clist->id = list->id;
clist->use = list->use;
@@ -261,7 +262,7 @@ XListInputDevices(
clist = sclist;
nptr = (unsigned char *)any;
Nptr = (unsigned char *)Any;
-   for (i = 0; i < *ndevices; i++, clist++) {
+   for (i = 0; i < ndevices; i++, clist++) {
clist->name = (char *)Nptr;
memcpy(Nptr, nptr + 1, *nptr);
Nptr += (*nptr);
@@ -270,6 +271,8 @@ XListInputDevices(
}
 }
 
+*ndevices_return = ndevices;
+
   out:
 XFree((char *)slist);
 UnlockDisplay(dpy);
-- 
2.7.4

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH libXi] SizeClassInfo can return 0 even without an error

2016-10-12 Thread Peter Hutterer
On Fri, Oct 07, 2016 at 09:46:44PM +0200, Niels Ole Salscheider wrote:
> Catch the error case separately. This fixes a few crashes on my computer.

others have mentioned this already, but "fixes a few crashes" is too generic
for a commit message. you should always explain *why* you're doing something
in as much detail as required for someone to reproduce or at least
understand the issue. in a few months time, most of us will have forgotten
what this patch was about and then rely on the commit message to refresh our
memory.

> Signed-off-by: Niels Ole Salscheider 
> ---
>  src/XListDev.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/src/XListDev.c b/src/XListDev.c
> index f850cd0..d0c6bf2 100644
> --- a/src/XListDev.c
> +++ b/src/XListDev.c
> @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size)
>  return ((base_size + padsize - 1)/padsize) * padsize;
>  }
>  
> -static size_t
> -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
> +static int
> +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size)
>  {
> -int size = 0;
>  int j;
> +*size = 0;

happy to merge this with this bit removed and adjusted so that size only
changes in the success case. but I will need a commit message that explains
bits in more detail.

Cheers,
   Peter

>  for (j = 0; j < num_classes; j++) {
>  switch ((*any)->class) {
>  case KeyClass:
> -size += pad_to_xid(sizeof(XKeyInfo));
> +*size += pad_to_xid(sizeof(XKeyInfo));
>  break;
>  case ButtonClass:
> -size += pad_to_xid(sizeof(XButtonInfo));
> +*size += pad_to_xid(sizeof(XButtonInfo));
>  break;
>  case ValuatorClass:
>  {
>  xValuatorInfoPtr v;
>  
>  if (len < sizeof(v))
> -return 0;
> +return 1;
>  v = (xValuatorInfoPtr) *any;
> -size += pad_to_xid(sizeof(XValuatorInfo) +
> +*size += pad_to_xid(sizeof(XValuatorInfo) +
>  (v->num_axes * sizeof(XAxisInfo)));
>  break;
>  }
> @@ -101,11 +101,11 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int 
> num_classes)
>  break;
>  }
>  if ((*any)->length > len)
> -return 0;
> +return 1;
>  *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length);
>  }
>  
> -return size;
> +return 0;
>  }
>  
>  static void
> @@ -220,8 +220,7 @@ XListInputDevices(
>   sav_any = any;
>   end = (char *)list + rlen;
>   for (i = 0; i < *ndevices; i++, list++) {
> -s = SizeClassInfo(, end - (char *)any, 
> (int)list->num_classes);
> -if (!s)
> +if(SizeClassInfo(, end - (char *)any, 
> (int)list->num_classes, ))
>  goto out;
>  size += s;
>   }
> -- 
> 2.10.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH libXi] SizeClassInfo can return 0 even without an error

2016-10-12 Thread Peter Hutterer
On Fri, Oct 07, 2016 at 09:46:44PM +0200, Niels Ole Salscheider wrote:
> Catch the error case separately. This fixes a few crashes on my computer.

others have mentioned this already, but "fixes a few crashes" is too generic
for a commit message. you should always explain *why* you're doing something
in as much detail as required for someone to reproduce or at least
understand the issue. in a few months time, most of us will have forgotten
what this patch was about and then rely on the commit message to refresh our
memory.

> Signed-off-by: Niels Ole Salscheider 
> ---
>  src/XListDev.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/src/XListDev.c b/src/XListDev.c
> index f850cd0..d0c6bf2 100644
> --- a/src/XListDev.c
> +++ b/src/XListDev.c
> @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size)
>  return ((base_size + padsize - 1)/padsize) * padsize;
>  }
>  
> -static size_t
> -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
> +static int
> +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t *size)
>  {
> -int size = 0;
>  int j;
> +*size = 0;

happy to merge this with this bit removed and adjusted so that size only
changes in the success case. but I will need a commit message that explains
bits in more detail.

Cheers,
   Peter

>  for (j = 0; j < num_classes; j++) {
>  switch ((*any)->class) {
>  case KeyClass:
> -size += pad_to_xid(sizeof(XKeyInfo));
> +*size += pad_to_xid(sizeof(XKeyInfo));
>  break;
>  case ButtonClass:
> -size += pad_to_xid(sizeof(XButtonInfo));
> +*size += pad_to_xid(sizeof(XButtonInfo));
>  break;
>  case ValuatorClass:
>  {
>  xValuatorInfoPtr v;
>  
>  if (len < sizeof(v))
> -return 0;
> +return 1;
>  v = (xValuatorInfoPtr) *any;
> -size += pad_to_xid(sizeof(XValuatorInfo) +
> +*size += pad_to_xid(sizeof(XValuatorInfo) +
>  (v->num_axes * sizeof(XAxisInfo)));
>  break;
>  }
> @@ -101,11 +101,11 @@ SizeClassInfo(xAnyClassPtr *any, size_t len, int 
> num_classes)
>  break;
>  }
>  if ((*any)->length > len)
> -return 0;
> +return 1;
>  *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length);
>  }
>  
> -return size;
> +return 0;
>  }
>  
>  static void
> @@ -220,8 +220,7 @@ XListInputDevices(
>   sav_any = any;
>   end = (char *)list + rlen;
>   for (i = 0; i < *ndevices; i++, list++) {
> -s = SizeClassInfo(, end - (char *)any, 
> (int)list->num_classes);
> -if (!s)
> +if(SizeClassInfo(, end - (char *)any, 
> (int)list->num_classes, ))
>  goto out;
>  size += s;
>   }
> -- 
> 2.10.1
> 
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Handle animated cursor on shared sprite

2016-10-12 Thread Emil Velikov
Hi Pierre,

On 12 October 2016 at 10:05, Pierre Ossman  wrote:
> Sprites (and hence cursors) can be shared between multiple devices.
> However the animation code was not prepared for this and could wind
> up in a case where it would continue to animate a free:d cursor.
> ---
>  include/inputstr.h | 14 -
>  render/animcur.c   | 85 
> +-
>  2 files changed, 51 insertions(+), 48 deletions(-)
>
> diff --git a/include/inputstr.h b/include/inputstr.h
> index 568f5f9..a485f5e 100644
> --- a/include/inputstr.h
> +++ b/include/inputstr.h
> @@ -246,6 +246,12 @@ typedef struct _SpriteRec {
>  ScreenPtr pEnqueueScreen;
>  ScreenPtr pDequeueScreen;
>  +/* keep states for animated cursor */
> +struct {
> +ScreenPtr pScreen;
> +int elt;
> +CARD32 time;
> +} anim;
>  } SpriteRec;
Note that this changes the ABI so it might not be good for the point
releases. Unless one wants to make things extra 'fun' for binary only
drivers ;-)

Haven't looked at the patch in detail, so I cannot comment the issue
can be fixed without breaking the ABI.

-Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Handle animated cursor on shared sprite

2016-10-12 Thread walter harms


Am 12.10.2016 11:05, schrieb Pierre Ossman:
> Sprites (and hence cursors) can be shared between multiple devices.
> However the animation code was not prepared for this and could wind
> up in a case where it would continue to animate a free:d cursor.
> ---
>  include/inputstr.h | 14 -
>  render/animcur.c   | 85 
> +-
>  2 files changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/include/inputstr.h b/include/inputstr.h
> index 568f5f9..a485f5e 100644
> --- a/include/inputstr.h
> +++ b/include/inputstr.h
> @@ -246,6 +246,12 @@ typedef struct _SpriteRec {
>  ScreenPtr pEnqueueScreen;
>  ScreenPtr pDequeueScreen;
>  +/* keep states for animated cursor */
> +struct {
> +ScreenPtr pScreen;
> +int elt;
> +CARD32 time;
> +} anim;
>  } SpriteRec;
>   typedef struct _KeyClassRec {
> @@ -509,14 +515,6 @@ typedef struct _SpriteInfoRec {
>  DeviceIntPtr paired;/* The paired device. Keyboard if
> spriteOwner is TRUE, otherwise the
> pointer that owns the sprite. */
> -
> -/* keep states for animated cursor */
> -struct {
> -CursorPtr pCursor;
> -ScreenPtr pScreen;
> -int elt;
> -CARD32 time;
> -} anim;
>  } SpriteInfoRec, *SpriteInfoPtr;
>   /* device types */
> diff --git a/render/animcur.c b/render/animcur.c
> index 52e6b8b..fc87e0d 100644
> --- a/render/animcur.c
> +++ b/render/animcur.c
> @@ -142,35 +142,42 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void 
> *arg)
>  CARD32 soonest = ~0;   /* earliest time to wakeup again */
>   for (dev = inputInfo.devices; dev; dev = dev->next) {
> -if (IsPointerDevice(dev) && pScreen == 
> dev->spriteInfo->anim.pScreen) {
> -if (!activeDevice)
> -activeDevice = TRUE;
> -
> -if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) {
> -AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor);
> -int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt;
> -DisplayCursorProcPtr DisplayCursor;
> -
> -/*
> - * Not a simple Unwrap/Wrap as this
> - * isn't called along the DisplayCursor
> - * wrapper chain.
> - */
> -DisplayCursor = pScreen->DisplayCursor;
> -pScreen->DisplayCursor = as->DisplayCursor;
> -(void) (*pScreen->DisplayCursor) (dev,
> -  pScreen,
> -  ac->elts[elt].pCursor);
> -as->DisplayCursor = pScreen->DisplayCursor;
> -pScreen->DisplayCursor = DisplayCursor;
> -
> -dev->spriteInfo->anim.elt = elt;
> -dev->spriteInfo->anim.time = now + ac->elts[elt].delay;
> -}
> -
> -if (soonest > dev->spriteInfo->anim.time)
> -soonest = dev->spriteInfo->anim.time;
> +if (!IsPointerDevice(dev))
> +continue;
> +if (!dev->spriteInfo->spriteOwner)
> +continue;
> +if (!IsAnimCur(dev->spriteInfo->sprite->current))
> +continue;
> +if (pScreen != dev->spriteInfo->sprite->anim.pScreen)
> +continue;
> +
> +if (!activeDevice)
> +activeDevice = TRUE;

   why the check here ? just set.


> +
> +if ((INT32) (now - dev->spriteInfo->sprite->anim.time) >= 0) {
> +AnimCurPtr ac = GetAnimCur(dev->spriteInfo->sprite->current);
> +int elt = (dev->spriteInfo->sprite->anim.elt + 1) % ac->nelt;
> +DisplayCursorProcPtr DisplayCursor;
> +
> +/*
> + * Not a simple Unwrap/Wrap as this
> + * isn't called along the DisplayCursor
> + * wrapper chain.
> + */
> +DisplayCursor = pScreen->DisplayCursor;
> +pScreen->DisplayCursor = as->DisplayCursor;
> +(void) (*pScreen->DisplayCursor) (dev,
> +  pScreen,
> +  ac->elts[elt].pCursor);
> +as->DisplayCursor = pScreen->DisplayCursor;
> +pScreen->DisplayCursor = DisplayCursor;
> +
> +dev->spriteInfo->sprite->anim.elt = elt;
> +dev->spriteInfo->sprite->anim.time = now + ac->elts[elt].delay;
>  }
> +
> +if (soonest > dev->spriteInfo->sprite->anim.time)
> +soonest = dev->spriteInfo->sprite->anim.time;
>  }
>   if (activeDevice)
> @@ -192,17 +199,17 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr 
> pScreen, CursorPtr pCursor)
>   Unwrap(as, pScreen, DisplayCursor);
>  if (IsAnimCur(pCursor)) {
> -if (pCursor != pDev->spriteInfo->anim.pCursor) {
> +

[PATCH xserver] inputthread: Re-add fd to the inputThreadInfo->fds pollfd set when re-added

2016-10-12 Thread Hans de Goede
If the following call sequence happens:
1) InputThreadUnregisterDev(fd)
2) close(fd)
3) fd = open(...) /* returns same fd as just closed */
4) InputThreadRegisterDev(fd, ...)

Without InputThreadDoWork(); running in the mean time, then we would
keep the closed fd in the inputThreadInfo->fds pollfd set, rather then
removing it and adding the new one, causing some devices to not work
after a vt-switch when using xf86-input-evdev.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97880
Reported-and-tested-by: Mihail Konev 
Signed-off-by: Hans de Goede 
---
 os/inputthread.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/os/inputthread.c b/os/inputthread.c
index ab1559f..c20c21c 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -49,6 +49,7 @@ Bool InputThreadEnable = TRUE;
 
 typedef enum _InputDeviceState {
 device_state_added,
+device_state_re_added,
 device_state_running,
 device_state_removed
 } InputDeviceState;
@@ -206,8 +207,8 @@ InputThreadRegisterDev(int fd,
 if (dev) {
 dev->readInputProc = readInputProc;
 dev->readInputArgs = readInputArgs;
-/* Override possible unhandled state == device_state_removed */
-dev->state = device_state_running;
+if (dev->state == device_state_removed)
+dev->state = device_state_re_added;
 } else {
 dev = calloc(1, sizeof(InputThreadDevice));
 if (dev == NULL) {
@@ -344,6 +345,16 @@ InputThreadDoWork(void *arg)
 ospoll_listen(inputThreadInfo->fds, dev->fd, 
X_NOTIFY_READ);
 dev->state = device_state_running;
 break;
+case device_state_re_added:
+/* Device might use a new fd with the same number */
+ospoll_remove(inputThreadInfo->fds, dev->fd);
+ospoll_add(inputThreadInfo->fds, dev->fd,
+   ospoll_trigger_level,
+   InputReady,
+   dev);
+ospoll_listen(inputThreadInfo->fds, dev->fd, 
X_NOTIFY_READ);
+dev->state = device_state_running;
+break;
 case device_state_running:
 break;
 case device_state_removed:
-- 
2.9.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] os: add an optional offset to -displayfd parameter

2016-10-12 Thread Bernhard Miklautz
Hi,

On Mon, Jun 13, 2016 at 11:44:55PM +0700, Antoine Martin wrote:
> Reviewed-by: Antoine Martin 

quite some time past since my initial submit. What are the next steps to
get this merged? Did I miss anything?

Thank you.

So long,
Bernhard
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 3/3] configure.ac: remove --enable-aiglx option

2016-10-12 Thread Emil Velikov
On 11 October 2016 at 14:34, Jon Turney  wrote:
> On 10/10/2016 19:31, Mihail Konev wrote:
>>
>> Hello.
>>
>> On Mon Oct 10 14:59:52 UTC 2016, Jon Turney wrote:
>>>
>>> @@ -16,11 +20,10 @@ AM_CPPFLAGS = \
>>> -I$(top_srcdir)/hw/xfree86/os-support/bus \
>>> -I$(top_srcdir)/hw/xfree86/common \
>>> -I$(top_srcdir)/hw/xfree86/dri \
>>> +   -I$(top_srcdir)/hw/xfree86/dri2
>>> -I$(top_srcdir)/mi \
>>> -I$(top_srcdir)/present
>>
>>
>> Shouldn't there be a backslash?
>
>
> Absolutely!  Revised patch attached.
>
One could have kept the include path as-is or even wrap it in if DRI2
(like the original code). Either way, the updated patch looks correct
and is
Reviewed-by: Emil Velikov 

Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Handle animated cursor on shared sprite

2016-10-12 Thread Pierre Ossman

This was causing crashes in TigerVNC with Chrome/Chromium.

Regards
--
Pierre Ossman   Software Development
Cendio AB   https://cendio.com
Teknikringen 8  https://twitter.com/ThinLinc
583 30 Linköpinghttps://facebook.com/ThinLinc
Phone: +46-13-214600https://plus.google.com/+CendioThinLinc

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] Handle animated cursor on shared sprite

2016-10-12 Thread Pierre Ossman
Sprites (and hence cursors) can be shared between multiple devices.
However the animation code was not prepared for this and could wind
up in a case where it would continue to animate a free:d cursor.
---
 include/inputstr.h | 14 -
 render/animcur.c   | 85 +-
 2 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/include/inputstr.h b/include/inputstr.h
index 568f5f9..a485f5e 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -246,6 +246,12 @@ typedef struct _SpriteRec {
 ScreenPtr pEnqueueScreen;
 ScreenPtr pDequeueScreen;
 +/* keep states for animated cursor */
+struct {
+ScreenPtr pScreen;
+int elt;
+CARD32 time;
+} anim;
 } SpriteRec;
  typedef struct _KeyClassRec {
@@ -509,14 +515,6 @@ typedef struct _SpriteInfoRec {
 DeviceIntPtr paired;/* The paired device. Keyboard if
spriteOwner is TRUE, otherwise the
pointer that owns the sprite. */
-
-/* keep states for animated cursor */
-struct {
-CursorPtr pCursor;
-ScreenPtr pScreen;
-int elt;
-CARD32 time;
-} anim;
 } SpriteInfoRec, *SpriteInfoPtr;
  /* device types */
diff --git a/render/animcur.c b/render/animcur.c
index 52e6b8b..fc87e0d 100644
--- a/render/animcur.c
+++ b/render/animcur.c
@@ -142,35 +142,42 @@ AnimCurTimerNotify(OsTimerPtr timer, CARD32 now, void 
*arg)
 CARD32 soonest = ~0;   /* earliest time to wakeup again */
  for (dev = inputInfo.devices; dev; dev = dev->next) {
-if (IsPointerDevice(dev) && pScreen == dev->spriteInfo->anim.pScreen) {
-if (!activeDevice)
-activeDevice = TRUE;
-
-if ((INT32) (now - dev->spriteInfo->anim.time) >= 0) {
-AnimCurPtr ac = GetAnimCur(dev->spriteInfo->anim.pCursor);
-int elt = (dev->spriteInfo->anim.elt + 1) % ac->nelt;
-DisplayCursorProcPtr DisplayCursor;
-
-/*
- * Not a simple Unwrap/Wrap as this
- * isn't called along the DisplayCursor
- * wrapper chain.
- */
-DisplayCursor = pScreen->DisplayCursor;
-pScreen->DisplayCursor = as->DisplayCursor;
-(void) (*pScreen->DisplayCursor) (dev,
-  pScreen,
-  ac->elts[elt].pCursor);
-as->DisplayCursor = pScreen->DisplayCursor;
-pScreen->DisplayCursor = DisplayCursor;
-
-dev->spriteInfo->anim.elt = elt;
-dev->spriteInfo->anim.time = now + ac->elts[elt].delay;
-}
-
-if (soonest > dev->spriteInfo->anim.time)
-soonest = dev->spriteInfo->anim.time;
+if (!IsPointerDevice(dev))
+continue;
+if (!dev->spriteInfo->spriteOwner)
+continue;
+if (!IsAnimCur(dev->spriteInfo->sprite->current))
+continue;
+if (pScreen != dev->spriteInfo->sprite->anim.pScreen)
+continue;
+
+if (!activeDevice)
+activeDevice = TRUE;
+
+if ((INT32) (now - dev->spriteInfo->sprite->anim.time) >= 0) {
+AnimCurPtr ac = GetAnimCur(dev->spriteInfo->sprite->current);
+int elt = (dev->spriteInfo->sprite->anim.elt + 1) % ac->nelt;
+DisplayCursorProcPtr DisplayCursor;
+
+/*
+ * Not a simple Unwrap/Wrap as this
+ * isn't called along the DisplayCursor
+ * wrapper chain.
+ */
+DisplayCursor = pScreen->DisplayCursor;
+pScreen->DisplayCursor = as->DisplayCursor;
+(void) (*pScreen->DisplayCursor) (dev,
+  pScreen,
+  ac->elts[elt].pCursor);
+as->DisplayCursor = pScreen->DisplayCursor;
+pScreen->DisplayCursor = DisplayCursor;
+
+dev->spriteInfo->sprite->anim.elt = elt;
+dev->spriteInfo->sprite->anim.time = now + ac->elts[elt].delay;
 }
+
+if (soonest > dev->spriteInfo->sprite->anim.time)
+soonest = dev->spriteInfo->sprite->anim.time;
 }
  if (activeDevice)
@@ -192,17 +199,17 @@ AnimCurDisplayCursor(DeviceIntPtr pDev, ScreenPtr 
pScreen, CursorPtr pCursor)
  Unwrap(as, pScreen, DisplayCursor);
 if (IsAnimCur(pCursor)) {
-if (pCursor != pDev->spriteInfo->anim.pCursor) {
+if (pDev->spriteInfo->spriteOwner &&
+(pCursor != pDev->spriteInfo->sprite->current)) {
 AnimCurPtr ac = GetAnimCur(pCursor);
  ret = (*pScreen->DisplayCursor)
 (pDev, pScreen, ac->elts[0].pCursor);
 if (ret) {
-pDev->spriteInfo->anim.elt = 0;
-