[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/3] util/main-loop: Fix maximum number of wait objects fo
From: |
Bin Meng |
Subject: |
Re: [PATCH v3 1/3] util/main-loop: Fix maximum number of wait objects for win32 |
Date: |
Sun, 25 Sep 2022 09:07:15 +0800 |
Hi Paolo,
On Tue, Sep 13, 2022 at 5:52 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Aug 24, 2022 at 12:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> The maximum number of wait objects for win32 should be
>> MAXIMUM_WAIT_OBJECTS, not MAXIMUM_WAIT_OBJECTS + 1.
>>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> ---
>>
>> Changes in v3:
>> - move the check of adding the same HANDLE twice to a separete patch
>>
>> Changes in v2:
>> - fix the logic in qemu_add_wait_object() to avoid adding
>> the same HANDLE twice
>>
>> util/main-loop.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/util/main-loop.c b/util/main-loop.c
>> index f00a25451b..cb018dc33c 100644
>> --- a/util/main-loop.c
>> +++ b/util/main-loop.c
>> @@ -363,10 +363,10 @@ void qemu_del_polling_cb(PollingFunc *func, void
>> *opaque)
>> /* Wait objects support */
>> typedef struct WaitObjects {
>> int num;
>> - int revents[MAXIMUM_WAIT_OBJECTS + 1];
>> - HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
>> - WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
>> - void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
>> + int revents[MAXIMUM_WAIT_OBJECTS];
>> + HANDLE events[MAXIMUM_WAIT_OBJECTS];
>> + WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS];
>> + void *opaque[MAXIMUM_WAIT_OBJECTS];
>> } WaitObjects;
>>
>> static WaitObjects wait_objects = {0};
>> @@ -395,6 +395,9 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
>> *func, void *opaque)
>> if (w->events[i] == handle) {
>> found = 1;
>> }
>> + if (i == MAXIMUM_WAIT_OBJECTS - 1) {
>> + break;
>> + }
>
>
> hmm
>
>>
>> if (found) {
>> w->events[i] = w->events[i + 1];
>> w->func[i] = w->func[i + 1];
>
>
> The way deletion works is by moving the i+1 element (which is always zeroed
> for i == MAXIMUM_WAIT_OBJECTS) to i.
>
> After your patch, for i == MAXIMUM_WAIT_OBJECTS, we no longer clear the last
> value, and instead rely simply on updated w->num:
>
> if (found) {
> w->num--;
> }
>
> So your patch looks ok to me, but I prefer the current code.
>
> Paolo, what do you say?
Ping?
Regards,
Bin