Re: [PATCH v3 1/3] util/main-loop: Fix maximum number of wait objects for win32

[Bin Meng]

+more people

On Mon, Oct 3, 2022 at 6:21 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Paolo,
>
> On Sun, Sep 25, 2022 at 9:07 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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?
> >
>
> Ping?
>

Could this series be merged? Thanks,

Regards,
Bin