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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 13/09/2014 04:22, TeLeMan ha scritto:
> On Fri, Sep 12, 2014 at 6:05 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 12/09/2014 03:39, TeLeMan ha scritto:
>>> On Wed, Jul 9, 2014 at 5:53 PM, Paolo Bonzini <pbonzini@redhat.com> 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.
> 
> WAIT_ABANDONED_0 & WAIT_TIMEOUT & WAIT_FAILED are larger than
> MAXIMUM_WAIT_OBJECTS.

WAIT_ABANDONED_0 and WAIT_FAILED cannot happen, but you're right about
WAIT_TIMEOUT.  Are you going to send a patch?

Paolo

>> 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];
>>
> 
>