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

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

On 19/11/12 19:39, Manuel Bouyer wrote:
> 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.

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. I will move the initialization upper.