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

[Roger Pau Monné <roger.pau%citrix.com@localhost>]

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

From: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= <roger.pau%citrix.com@localhost>
To: Manuel Bouyer <bouyer%antioche.eu.org@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: Tue, 20 Nov 2012 10:26:29 +0100

 On 19/11/12 20:28, Manuel Bouyer wrote:
 > On Mon, Nov 19, 2012 at 07:49:44PM +0100, Roger Pau Monné wrote:
 >>> 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.
 >>
 >> I guess because if "repeated" is not initialized, it is not even
 >> checked, since the first part of the if condition "if (!found)" will
 >> always be true.
 > 
 > Right. But I guess the waring would trip with different optimisation
 > options.
 
 Thanks for the review and please take a look at the following patch:
 
 ---
 diff --git a/sys/arch/xen/xenbus/xenbus_xs.c b/sys/arch/xen/xenbus/xenbus_xs.c
 index c09e7c4..589e7e1 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,
 @@ -796,14 +796,24 @@ process_msg(void)
                msg->u.watch.handle = find_watch(
                    msg->u.watch.vec[XS_WATCH_TOKEN]);
                found = (msg->u.watch.handle != NULL);
 +              repeated = false;
                if (found) {
                        mutex_enter(&watch_events_lock);
 -                      SIMPLEQ_INSERT_TAIL(&watch_events, msg, msg_next);
 -                      cv_broadcast(&watch_cv);
 +                      /* 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);
                }