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

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

The following reply was made to PR port-xen/47193; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Roger Pau =?iso-8859-1?Q?Monn=E9?= <roger.pau%citrix.com@localhost>
Cc: "Cherry G.Mathew" <cherry%zyx.in@localhost>,
        "gnats-bugs%NetBSD.org@localhost" <gnats-bugs%NetBSD.org@localhost>,
        "port-xen-maintainer%netbsd.org@localhost" 
<port-xen-maintainer%NetBSD.org@localhost>,
        "netbsd-bugs%netbsd.org@localhost" <netbsd-bugs%NetBSD.org@localhost>
Subject: Re: port-xen/47193: xenwatch handlers are added twice to the list of
 pending watches
Date: Mon, 19 Nov 2012 19:39:52 +0100

 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
 --