On Fri, May 27, 2016 at 01:58:56AM -0700, Keith Packard wrote: > Michel Dänzer <mic...@daenzer.net> writes: > > > On 27.05.2016 08:08, Keith Packard wrote: > >> > >> commit f84703b50cc908a127f4ad923ebbf56f8f244c0d > >> Author: Keith Packard <kei...@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 <kei...@keithp.com> > >> Reviewed-by: Peter Hutterer <peter.hutte...@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 <kei...@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 <kei...@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.hutte...@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 _______________________________________________ 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