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

[Paolo Bonzini]

Il 15/09/2014 03:18, TeLeMan ha scritto:
> On Sat, Sep 13, 2014 at 6:33 PM, Paolo Bonzini <address@hidden> wrote:
>> Il 13/09/2014 04:22, TeLeMan ha scritto:
>>> On Fri, Sep 12, 2014 at 6:05 PM, Paolo Bonzini <address@hidden> wrote:
>>>> 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.
>>>
>>> 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?
> 
> No, because I was rejected to submit the patch, so I just report the issues.

If this is because of the pseudonym, I think what was done with Blue
Swirl was that he told someone his real name.  You can do the same if
you wish to contribute more than just bug reports.  Can you review
patches too?  The "Reviewed-by" can be done with a pseudonym.

Paolo