On Wed, Aug 10, 2022 at 11:57 PM Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > > > > On Wed, Aug 10, 2022 at 7:20 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> On Wed, Aug 10, 2022 at 1:06 AM Marc-André Lureau >> <marcandre.lur...@gmail.com> wrote: >> > >> > Hi >> > >> > On Tue, Aug 9, 2022 at 8:43 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> >> >> From: Bin Meng <bin.m...@windriver.com> >> >> >> >> The maximum number of wait objects for win32 should be >> >> MAXIMUM_WAIT_OBJECTS, not MAXIMUM_WAIT_OBJECTS + 1. >> >> >> >> Fix the logic in qemu_add_wait_object() to avoid adding >> >> the same HANDLE twice. >> >> >> > >> > Please make that a separate patch. >> > >> >> >> >> Signed-off-by: Bin Meng <bin.m...@windriver.com> >> >> --- >> >> >> >> Changes in v2: >> >> - fix the logic in qemu_add_wait_object() to avoid adding >> >> the same HANDLE twice >> >> >> > >> > Still NACK, did you understand my argument about array bounds? >> > >> > "if (found)" will access the arrays at position i+1 == >> > MAXIMUM_WAIT_OBJECTS. We need the +1 for that logic to work without OOB >> > access. >> > >> >> The delete logic was updated in v2. If position is at >> MAXIMUM_WAIT_OBJECTS - 1, the loop will break. > > > Ah I missed that. That new condition looks wrong to me. Not only it is > redundant with the loop condition check if w->num == MAXIMUM_WAIT_OBJECTS
Did you mean the "w->num >= MAXIMUM_WAIT_OBJECTS" check in qemu_add_wait_object()? It's necessary because it prevents the OOB access to the array. > But you still access the array at MAXIMUM_WAIT_OBJECTS index, which requires > arrays of MAXIMUM_WAIT_OBJECTS+1 size, since it's 0-indexed.. There is no OOB access in either add or delete. Am I missing anything? > > Unless I say crap, which happens sometime :) > Regards, Bin