NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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);
                }




Home | Main Index | Thread Index | Old Index