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.
> util/main-loop.c | 43 +++++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/util/main-loop.c b/util/main-loop.c
> index f00a25451b..66b2ae2800 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -363,37 +363,56 @@ 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};
>
> int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void
> *opaque)
> {
> + int i;
> + bool found = false;
> WaitObjects *w = &wait_objects;
> +
> if (w->num >= MAXIMUM_WAIT_OBJECTS) {
> return -1;
> }
> - w->events[w->num] = handle;
> - w->func[w->num] = func;
> - w->opaque[w->num] = opaque;
> - w->revents[w->num] = 0;
> - w->num++;
> +
> + for (i = 0; i < w->num; i++) {
> + /* if the same handle is added twice, newer overwrites older */
> + if (w->events[i] == handle) {
> + found = true;
> + break;
> + }
> + }
> +
> + w->events[i] = handle;
> + w->func[i] = func;
> + w->opaque[i] = opaque;
> + w->revents[i] = 0;
> +
> + if (!found) {
> + w->num++;
> + }
> +
> return 0;
> }
>
> void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void
> *opaque)
> {
> - int i, found;
> + int i;
> + bool found = false;
> WaitObjects *w = &wait_objects;
>
> - found = 0;
> for (i = 0; i < w->num; i++) {
> if (w->events[i] == handle) {
> - found = 1;
> + found = true;
> + }
> + if (i == MAXIMUM_WAIT_OBJECTS - 1) {
> + break;
> }
> if (found) {
> w->events[i] = w->events[i + 1];
> --
> 2.34.1
>
>
--
Marc-André Lureau
