Re: port-xen/47193: xenwatch handlers are added twice to the list of pending watches

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sat, Nov 17, 2012 at 09:26:46AM +0100, Roger Pau Monné wrote:
> [...]
> diff --git a/sys/arch/xen/xenbus/xenbus_xs.c b/sys/arch/xen/xenbus/xenbus_xs.c
> index c09e7c4..1405807 100644
> --- a/sys/arch/xen/xenbus/xenbus_xs.c
> +++ b/sys/arch/xen/xenbus/xenbus_xs.c
> @@ -751,7 +751,7 @@ xenwatch_thread(void *unused)
>  static int
>  process_msg(void)
>  {
> -     struct xs_stored_msg *msg;
> +     struct xs_stored_msg *msg, *s_msg;
>       char *body;
>       int err;
>  
> @@ -782,7 +782,7 @@ process_msg(void)
>       body[msg->hdr.len] = '\0';
>  
>       if (msg->hdr.type == XS_WATCH_EVENT) {
> -             bool found;
> +             bool found, repeated;
>  
>               DPRINTK("process_msg: XS_WATCH_EVENT");
>               msg->u.watch.vec = split(body, msg->hdr.len,
> @@ -798,12 +798,22 @@ process_msg(void)
>               found = (msg->u.watch.handle != NULL);
>               if (found) {
>                       mutex_enter(&watch_events_lock);
> -                     SIMPLEQ_INSERT_TAIL(&watch_events, msg, msg_next);
> -                     cv_broadcast(&watch_cv);
> +                     repeated = false;
> +                     /* Don't add duplicate events to the queue of pending 
> watches */
> +                     SIMPLEQ_FOREACH(s_msg, &watch_events, msg_next) {
> +                             if (s_msg->u.watch.handle == 
> msg->u.watch.handle) {
> +                                     repeated = true;
> +                                     break;
> +                             }
> +                     }
> +                     if (!repeated) {
> +                             SIMPLEQ_INSERT_TAIL(&watch_events, msg, 
> msg_next);
> +                             cv_broadcast(&watch_cv);
> +                     }
>                       mutex_exit(&watch_events_lock);
>               }
>               mutex_exit(&watches_lock);
> -             if (!found) {
> +             if (!found || repeated) {
>                       free(msg->u.watch.vec, M_DEVBUF);
>                       free(msg, M_DEVBUF);
>               }

I don't know why gcc doesn't complain about repeated being possibly
uninitialised here. AFAIK repeated is initialised only in the found case.
repeated = false should probably be moved upper.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--