Re: [PATCH v3 2/3] util/main-loop: Avoid adding the same HANDLE twice

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 24, 2022 at 04:52:30PM +0800, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Fix the logic in qemu_add_wait_object() to avoid adding the same
> HANDLE twice, as the behavior is undefined when passing an array
> that contains same HANDLEs to WaitForMultipleObjects() API.

Have you encountered this problem in the real world, or is this
just a flaw you spotted through code inspection ?

Essentially I'm wondering if there's any known caller that is
making this mistake of adding it twice ?

> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
> Changes in v3:
> - new patch: avoid adding the same HANDLE twice
> 
>  include/qemu/main-loop.h |  2 ++
>  util/main-loop.c         | 10 ++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> index c50d1b7e3a..db8d380550 100644
> --- a/include/qemu/main-loop.h
> +++ b/include/qemu/main-loop.h
> @@ -157,6 +157,8 @@ typedef void WaitObjectFunc(void *opaque);
>   * in the main loop's calls to WaitForMultipleObjects.  When the handle
>   * is in a signaled state, QEMU will call @func.
>   *
> + * If the same HANDLE is added twice, this function returns -1.
> + *
>   * @handle: The Windows handle to be observed.
>   * @func: A function to be called when @handle is in a signaled state.
>   * @opaque: A pointer-size value that is passed to @func.
> diff --git a/util/main-loop.c b/util/main-loop.c
> index cb018dc33c..dae33a8daf 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -373,10 +373,20 @@ static WaitObjects wait_objects = {0};
>  
>  int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
>  {
> +    int i;
>      WaitObjects *w = &wait_objects;
> +
>      if (w->num >= MAXIMUM_WAIT_OBJECTS) {
>          return -1;
>      }
> +
> +    for (i = 0; i < w->num; i++) {
> +        /* check if the same handle is added twice */
> +        if (w->events[i] == handle) {
> +            return -1;
> +        }
> +    }
> +
>      w->events[w->num] = handle;
>      w->func[w->num] = func;
>      w->opaque[w->num] = opaque;
> -- 
> 2.34.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|