[PATCH 1/8] Eliminate bogus event resizing.

[Peter Hutterer]

On Thu, Sep 17, 2009 at 06:14:31PM -0700, Keith Packard wrote:
> Now that all event queues hold internal events only, they never need
> to be resized. Resizing them led to memory corruption as they would
> get sized for an appropriate xEvent, not an internal event.
> 
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>  dix/devices.c   |    9 ---------
>  dix/getevents.c |   24 ------------------------
>  include/input.h |    3 ---
>  mi/mi.h         |    4 +---
>  mi/mieq.c       |   21 +++++++++++++--------
>  5 files changed, 14 insertions(+), 47 deletions(-)
> 
> diff --git a/dix/devices.c b/dix/devices.c
> index 0be3d58..0aece05 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -282,7 +282,6 @@ EnableDevice(DeviceIntPtr dev, BOOL sendevent)
>      int ret;
>      DeviceIntPtr other;
>      int evsize  = sizeof(xEvent);
> -    int listlen;
>      EventListPtr evlist;
>      BOOL enabled;
>      int flags[MAXDEVICES] = {0};
> @@ -325,13 +324,6 @@ EnableDevice(DeviceIntPtr dev, BOOL sendevent)
>  
>      evsize += SizeDeviceClasses(dev);
>  
> -    listlen = GetEventList(&evlist);
> -    OsBlockSignals();
> -    SetMinimumEventSize(evlist, listlen, evsize);
> -    mieqResizeEvents(evsize);
> -    OsReleaseSignals();
> -
> -
>      if ((*prev != dev) || !dev->inited ||
>  	((ret = (*dev->deviceProc)(dev, DEVICE_ON)) != Success)) {
>          ErrorF("[dix] couldn't enable device %d\n", dev->id);
> @@ -2390,7 +2382,6 @@ AttachDevice(ClientPtr client, DeviceIntPtr dev, DeviceIntPtr master)
>  
>              /* XXX: reset master back to defaults */
>              event = InitEventList(1);
> -            SetMinimumEventSize(event, 1, sizeof(DeviceChangedEvent));
>              CreateClassesChangedEvent(event, oldmaster, oldmaster,
>                                        DEVCHANGE_POINTER_EVENT | DEVCHANGE_KEYBOARD_EVENT);
>              XISendDeviceChangedEvent(oldmaster, oldmaster,

I amended the patch here, these lines are gone in master. Nothing special,
but gets rid of evsize and evlist.

> diff --git a/dix/getevents.c b/dix/getevents.c
> index 2912c1e..d622d07 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -968,30 +968,6 @@ InitEventList(int num_events)
>  }
>  
>  /**
> - * Allocs min_size memory for each event in the list.
> - */
> -void
> -SetMinimumEventSize(EventListPtr list, int num_events, int min_size)
> -{
> -    if (!list)
> -        return;
> -
> -    while(num_events--)
> -    {
> -        if (list[num_events].evlen < min_size)
> -        {
> -            list[num_events].evlen = min_size;
> -            list[num_events].event = realloc(list[num_events].event, min_size);
> -            if (!list[num_events].event)
> -            {
> -                FatalError("[dix] Failed to set event list's "
> -                        "min_size to %d.\n", min_size);
> -            }
> -        }
> -    }
> -}
> -
> -/**
>   * Free an event list.
>   *
>   * @param list The list to be freed.
> diff --git a/include/input.h b/include/input.h
> index 7ab5e9d..2fe6f92 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -409,9 +409,6 @@ extern _X_EXPORT int GetMaximumEventsNum(void);
>  
>  extern _X_EXPORT int GetEventList(EventListPtr* list);
>  extern _X_EXPORT EventListPtr InitEventList(int num_events);
> -extern _X_EXPORT void SetMinimumEventSize(EventListPtr list,
> -                                int num_events,
> -                                int min_size);
>  extern _X_EXPORT void FreeEventList(EventListPtr list, int num_events);
>  
>  extern void CreateClassesChangedEvent(EventListPtr event,
> diff --git a/mi/mi.h b/mi/mi.h
> index 3db8bfc..812edce 100644
> --- a/mi/mi.h
> +++ b/mi/mi.h
> @@ -196,9 +196,7 @@ extern _X_EXPORT Bool mieqInit(
>      void
>  );
>  
> -extern _X_EXPORT void mieqResizeEvents(
> -    int /* min_size */
> -);
> +extern _X_EXPORT void mieqFini(void);
>  
>  extern _X_EXPORT void mieqEnqueue(
>      DeviceIntPtr /*pDev*/,
> diff --git a/mi/mieq.c b/mi/mieq.c
> index 1b81e4d..0b64882 100644
> --- a/mi/mieq.c
> +++ b/mi/mieq.c
> @@ -110,24 +110,29 @@ mieqInit(void)
>          miEventQueue.handlers[i] = NULL;
>      for (i = 0; i < QUEUE_SIZE; i++)
>      {
> -        EventListPtr evlist = InitEventList(1);
> -        if (!evlist)
> -            FatalError("Could not allocate event queue.\n");
> -        miEventQueue.events[i].events = evlist;
> +	if (miEventQueue.events[i].events == NULL) {
> +	    EventListPtr evlist = InitEventList(1);
> +	    if (!evlist)
> +		FatalError("Could not allocate event queue.\n");
> +	    miEventQueue.events[i].events = evlist;
> +	}
>      }
>  
>      SetInputCheck(&miEventQueue.head, &miEventQueue.tail);
>      return TRUE;
>  }
>  
> -/* Ensure all events in the EQ are at least size bytes. */
>  void
> -mieqResizeEvents(int min_size)
> +mieqFini(void)
>  {
>      int i;
> -
>      for (i = 0; i < QUEUE_SIZE; i++)
> -        SetMinimumEventSize(miEventQueue.events[i].events, 1, min_size);
> +    {
> +	if (miEventQueue.events[i].events != NULL) {
> +	    FreeEventList(miEventQueue.events[i].events, 1);
> +	    miEventQueue.events[i].events = NULL;
> +	}
> +    }
>  }
>  
>  /*
> -- 
> 1.6.3.3

ACK for the patch, looks good. I'll give it a bit more testing and push it. Thanks.

Cheers,
  Peter