Re: [Qemu-devel] [PATCH 10/10] aio-win32: add support for sockets

[Paolo Bonzini]

Il 12/09/2014 03:39, TeLeMan ha scritto:
> On Wed, Jul 9, 2014 at 5:53 PM, Paolo Bonzini <address@hidden> wrote:
>> diff --git a/aio-win32.c b/aio-win32.c
>> index 4542270..61e3d2d 100644
>> --- a/aio-win32.c
>> +++ b/aio-win32.c
>> +    bool was_dispatching, progress, have_select_revents, first;
> have_select_revents has no initial value.

Good catch here...

> 
>> @@ -183,6 +318,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
>>
>>      /* wait until next event */
>>      while (count > 0) {
>> +        HANDLE event;
>>          int ret;
>>
>>          timeout = blocking
>> @@ -196,13 +332,17 @@ bool aio_poll(AioContext *ctx, bool blocking)
>>          first = false;
>>
>>          /* if we have any signaled events, dispatch event */
>> -        if ((DWORD) (ret - WAIT_OBJECT_0) >= count) {
>> +        event = NULL;
>> +        if ((DWORD) (ret - WAIT_OBJECT_0) < count) {
>> +            event = events[ret - WAIT_OBJECT_0];
>> +        } else if (!have_select_revents) {
> 
> when (ret - WAIT_OBJECT_0) >= count and have_select_revents is true,
> the following events[ret - WAIT_OBJECT_0] will be overflowed.

... this instead is not a problem, ret - WAIT_OBJECT_0 can be at most
equal to count, and events[] is declared with MAXIMUM_WAIT_OBJECTS + 1
places.  So the

        events[ret - WAIT_OBJECT_0] = events[--count];

is equal to

        events[count] = events[count - 1];
        --count;

and this is harmless.

Paolo

>>              break;
>>          }
>>
>> +        have_select_revents = false;
>>          blocking = false;
>>
>> -        progress |= aio_dispatch_handlers(ctx, events[ret - WAIT_OBJECT_0]);
>> +        progress |= aio_dispatch_handlers(ctx, event);
>>
>>          /* Try again, but only call each handler once.  */
>>          events[ret - WAIT_OBJECT_0] = events[--count];