xserver: Branch 'master' - 10 commits

[Peter Hutterer]

On Fri, May 27, 2016 at 01:58:56AM -0700, Keith Packard wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> 
> > On 27.05.2016 08:08, Keith Packard wrote:
> >> 
> >> commit f84703b50cc908a127f4ad923ebbf56f8f244c0d
> >> Author: Keith Packard <keithp at keithp.com>
> >> Date:   Tue Dec 8 14:20:21 2015 -0800
> >> 
> >>     dix: Reallocate touchpoint buffer at input event time [v2]
> >>     
> >>     Now that input is threaded, malloc can be used at event time to resize
> >>     the touchpoint buffer as needed.x
> >>     
> >>     v2: Remove "Need to grow the queue means dropping events."
> >>         from comment as it no longer applies. (Peter Hutterer)
> >>     
> >>     Signed-off-by: Keith Packard <keithp at keithp.com>
> >>     Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
> >
> > This change broke make check for me, specifically test/touch:
> >
> > touch: ../../test/touch.c:62: touch_grow_queue: Assertion
> > `TouchBeginDDXTouch(&dev, 1234) == ((void *)0)' failed.
> 
> yeah, that test actually checks for the old (broken) X server behaviour
> of dropping touch events that overfill the buffer. The X server doesn't
> drop them anymore as it can call malloc while reading events now.
> 

> From 85fef2e05775beec41eaecf9ee517bd6ffeef858 Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp at keithp.com>
> Date: Fri, 27 May 2016 01:56:39 -0700
> Subject: [PATCH xserver] test: Make touch test reflect new ability to realloc
>  touch array
> 
> Threaded input allows the input code to call malloc while processing
> events. In this case, that's in the middle of processing touch events
> and needing to resize the touch buffer.
> 
> This test was expecting the old behaviour where touch points would get
> dropped if the buffer was full. The fix is to check for the new
> behaviour instead.
> 
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>  test/touch.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/test/touch.c b/test/touch.c
> index 981c694..e0c1bcc 100644
> --- a/test/touch.c
> +++ b/test/touch.c
> @@ -58,9 +58,8 @@ touch_grow_queue(void)
>          dev.last.touches[i].client_id = i * 2;
>      }
>  
> -    /* no more space, should've scheduled a workproc */
> -    assert(TouchBeginDDXTouch(&dev, 1234) == NULL);
> -    ProcessWorkQueue();
> +    /* no more space, should've reallocated and succeeded */
> +    assert(TouchBeginDDXTouch(&dev, 1234) != NULL);
>  
>      new_size = size + size / 2 + 1;
>      assert(dev.last.num_touches == new_size);
> @@ -74,8 +73,12 @@ touch_grow_queue(void)
>          assert(t->client_id == i * 2);
>      }
>  
> +    assert(dev.last.touches[size].active == TRUE);
> +    assert(dev.last.touches[size].ddx_id == 1234);
> +    assert(dev.last.touches[size].client_id == 1);
> +
>      /* make sure those are zero-initialized */
> -    for (i = size; i < new_size; i++) {
> +    for (i = size + 1; i < new_size; i++) {
>          DDXTouchPointInfoPtr t = &dev.last.touches[i];
>  
>          assert(t->active == FALSE);
> @@ -136,22 +139,20 @@ touch_find_ddxid(void)
>      for (i = 0; i < size; i++)
>          dev.last.touches[i].active = TRUE;
>  
> -    /* Try to create more, fail */
> +    /* Try to create more, succeed */
>      ti = TouchFindByDDXID(&dev, 30, TRUE);
> -    assert(ti == NULL);
> +    assert(ti != NULL);
>      ti = TouchFindByDDXID(&dev, 30, TRUE);
> -    assert(ti == NULL);
> -    /* make sure we haven't resized, we're in the signal handler */
> -    assert(dev.last.num_touches == size);
> +    assert(ti != NULL);

this should check that the two ti pointers are the same, not just for NULL.

> +    /* make sure we have resized */
> +    assert(dev.last.num_touches == size + 3);

This needs a comment, I had to wonder why it was +3 when we only tried to
add 2 more touches over the queue size. turns out it's because we resize
with (size/2 + 1) which for a size of 5 ends up being 3. So this would be a
lot better as:

   assert(dev.last.num_touches == 8); /* EQ size grows from 5 to 8 */

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

Cheers,
   Peter

>  
>      /* stop one touchpoint, try to create, succeed */
>      dev.last.touches[2].active = FALSE;
> -    ti = TouchFindByDDXID(&dev, 30, TRUE);
> +    ti = TouchFindByDDXID(&dev, 35, TRUE);
>      assert(ti == &dev.last.touches[2]);
> -    /* but still grow anyway */
> -    ProcessWorkQueue();
>      ti = TouchFindByDDXID(&dev, 40, TRUE);
> -    assert(ti == &dev.last.touches[size]);
> +    assert(ti == &dev.last.touches[size+1]);


>  
>      free(dev.name);
>  }
> -- 
> 2.8.1
> 

> 
> -- 
> -keith