[PATCH 1/2 v4] mieq: Provide better adaptability and diagnostics during mieq overflow

[Jeremy Huddleston]

On Oct 16, 2011, at 9:44 PM, Keith Packard wrote:

> On Sun, 16 Oct 2011 21:16:09 -0700, Jeremy Huddleston <jeremyhu at apple.com> wrote:
> 
>> +    /* We block signals, so SIGIO does trigger mieqEnqueue to write to the
>> +     * queue as we're modifying it.
>> +     */
>> +    OsBlockSignals();
> 
> this comment is incorrect; the goal is to avoid having SIGIO come in and
> smash the queue.

Yeah, that's essentially what I meant.  Perhaps staring at code too long has made my English suffer.

> Tricky, but I think it's right. Might be easier to read
> if each argument to memcpy were on a separate line?
> 
> +    memcpy(new_events,
> +           &eventQueue->events[miEventQueue.head],
> +           (eventQueue->nevents - eventQueue->head) * sizeof(EventRec));
> +
> +    memcpy(&new_events[eventQueue->nevents - eventQueue->head],
> +           eventQueue->events,
> +           (eventQueue->head) * sizeof(EventRec));
> 
> You could give a name to 'nevents - head'? Not because the compiler
> won't figure it out, but just because it might be clearer how the new
> event queue is getting constructed.

n_events_at_tail ? I'm not really sure what to call it.

> 
>> +
>> +    /* Initialize the new portion */
>> +    for (i = eventQueue->nevents; i < new_nevents; i++) {
>> +        InternalEvent* evlist = InitEventList(1);
>> +        if (!evlist) {
>> +            free(new_events);
>> +            OsReleaseSignals();
>> +            return FALSE;
>> +        }
> 
> You'll have to free all of the InternalEvents allocated up to the point
> of failure.

Yeah...

> 
>> -    miEventQueue.head = miEventQueue.tail = 0;
>> +    memset(&miEventQueue, 0, sizeof(miEventQueue));
> 
> Looks like you're also fixing potential bugs with uninitialized values
> across server reset; that seems like a different change which isn't
> directly related to this fix. I'd post that change first and layer your
> resize stuff on top of it.

It's not really any different.  I'm just doing an optimization there... Rather than just setting everything to 0 individually, just do one big memset.

> Oh. One big problem -- mieqEnqueue can be called from a signal handler,
> in which case calling 'malloc' is a big no-no.

malloc isn't being called from within mieqEnqueue.  The resize is being done in mieqProcessInputEvents.

> I suspect you'll need to
> make this whole mess conditional for systems which have credible
> threaded input.

No, we just try to resize in mieqProcessInputEvents *before* it fills up.