Patchwork [1/8] notifier: switch to QLIST

login
register
mail settings
Submitter Paolo Bonzini
Date Jan. 2, 2012, 6 p.m.
Message ID <1325527237-24146-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/133895/
State New
Headers show

Comments

Paolo Bonzini - Jan. 2, 2012, 6 p.m.
Notifiers do not need to access both ends of the list, and using
a QLIST also simplifies the API.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	I do plan to exploit the simplified API in a future patch...

 input.c      |    2 +-
 migration.c  |    2 +-
 notify.c     |   10 +++++-----
 notify.h     |    8 ++++----
 qemu-timer.c |    2 +-
 vl.c         |    2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)
Stefan Hajnoczi - Jan. 3, 2012, 11:54 a.m.
On Mon, Jan 02, 2012 at 07:00:30PM +0100, Paolo Bonzini wrote:
>  void notifier_list_add(NotifierList *list, Notifier *notifier)
>  {
> -    QTAILQ_INSERT_HEAD(&list->notifiers, notifier, node);
> +    QLIST_INSERT_HEAD(&list->notifiers, notifier, node);
>  }
>  
> -void notifier_list_remove(NotifierList *list, Notifier *notifier)
> +void notifier_remove(Notifier *notifier)

Why introduce this asymmetry with notifier_list_add() and
notifier_remove()?  Please make the function names consistent.

Stefan
Paolo Bonzini - Jan. 3, 2012, 11:59 a.m.
On 01/03/2012 12:54 PM, Stefan Hajnoczi wrote:
> On Mon, Jan 02, 2012 at 07:00:30PM +0100, Paolo Bonzini wrote:
>>   void notifier_list_add(NotifierList *list, Notifier *notifier)
>>   {
>> -    QTAILQ_INSERT_HEAD(&list->notifiers, notifier, node);
>> +    QLIST_INSERT_HEAD(&list->notifiers, notifier, node);
>>   }
>>
>> -void notifier_list_remove(NotifierList *list, Notifier *notifier)
>> +void notifier_remove(Notifier *notifier)
>
> Why introduce this asymmetry with notifier_list_add() and
> notifier_remove()?  Please make the function names consistent.

Because notifier_list_add adds the notifier to a specific NotifierList; 
notifier_remove removes the notifier from whatever list it is in.

Normally whoever implements notifiers does not have access to the 
NotifierList, so there are wrappers for both notifier_list_add and 
notifier_list_remove.  This patch changes things so that the wrappers 
for notifier_remove are not needed anymore (though this series was 
already big enough, so I left the wrappers in).

Paolo
Stefan Hajnoczi - Jan. 3, 2012, 12:09 p.m.
On Tue, Jan 3, 2012 at 11:59 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/03/2012 12:54 PM, Stefan Hajnoczi wrote:
>>
>> On Mon, Jan 02, 2012 at 07:00:30PM +0100, Paolo Bonzini wrote:
>>>
>>>  void notifier_list_add(NotifierList *list, Notifier *notifier)
>>>  {
>>> -    QTAILQ_INSERT_HEAD(&list->notifiers, notifier, node);
>>> +    QLIST_INSERT_HEAD(&list->notifiers, notifier, node);
>>>  }
>>>
>>> -void notifier_list_remove(NotifierList *list, Notifier *notifier)
>>> +void notifier_remove(Notifier *notifier)
>>
>>
>> Why introduce this asymmetry with notifier_list_add() and
>> notifier_remove()?  Please make the function names consistent.
>
>
> Because notifier_list_add adds the notifier to a specific NotifierList;
> notifier_remove removes the notifier from whatever list it is in.
>
> Normally whoever implements notifiers does not have access to the
> NotifierList, so there are wrappers for both notifier_list_add and
> notifier_list_remove.  This patch changes things so that the wrappers for
> notifier_remove are not needed anymore (though this series was already big
> enough, so I left the wrappers in).

I see.

Stefan

Patch

diff --git a/input.c b/input.c
index 9ade63f..b618ea4 100644
--- a/input.c
+++ b/input.c
@@ -268,5 +268,5 @@  void qemu_add_mouse_mode_change_notifier(Notifier *notify)
 
 void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
 {
-    notifier_list_remove(&mouse_mode_notifiers, notify);
+    notifier_remove(notify);
 }
diff --git a/migration.c b/migration.c
index 412fdfe..0de907c 100644
--- a/migration.c
+++ b/migration.c
@@ -333,7 +333,7 @@  void add_migration_state_change_notifier(Notifier *notify)
 
 void remove_migration_state_change_notifier(Notifier *notify)
 {
-    notifier_list_remove(&migration_state_notifiers, notify);
+    notifier_remove(notify);
 }
 
 bool migration_is_active(MigrationState *s)
diff --git a/notify.c b/notify.c
index a6bac1f..ac05f91 100644
--- a/notify.c
+++ b/notify.c
@@ -16,24 +16,24 @@ 
 
 void notifier_list_init(NotifierList *list)
 {
-    QTAILQ_INIT(&list->notifiers);
+    QLIST_INIT(&list->notifiers);
 }
 
 void notifier_list_add(NotifierList *list, Notifier *notifier)
 {
-    QTAILQ_INSERT_HEAD(&list->notifiers, notifier, node);
+    QLIST_INSERT_HEAD(&list->notifiers, notifier, node);
 }
 
-void notifier_list_remove(NotifierList *list, Notifier *notifier)
+void notifier_remove(Notifier *notifier)
 {
-    QTAILQ_REMOVE(&list->notifiers, notifier, node);
+    QLIST_REMOVE(notifier, node);
 }
 
 void notifier_list_notify(NotifierList *list, void *data)
 {
     Notifier *notifier, *next;
 
-    QTAILQ_FOREACH_SAFE(notifier, &list->notifiers, node, next) {
+    QLIST_FOREACH_SAFE(notifier, &list->notifiers, node, next) {
         notifier->notify(notifier, data);
     }
 }
diff --git a/notify.h b/notify.h
index 54fc57c..03cf26c 100644
--- a/notify.h
+++ b/notify.h
@@ -21,22 +21,22 @@  typedef struct Notifier Notifier;
 struct Notifier
 {
     void (*notify)(Notifier *notifier, void *data);
-    QTAILQ_ENTRY(Notifier) node;
+    QLIST_ENTRY(Notifier) node;
 };
 
 typedef struct NotifierList
 {
-    QTAILQ_HEAD(, Notifier) notifiers;
+    QLIST_HEAD(, Notifier) notifiers;
 } NotifierList;
 
 #define NOTIFIER_LIST_INITIALIZER(head) \
-    { QTAILQ_HEAD_INITIALIZER((head).notifiers) }
+    { QLIST_HEAD_INITIALIZER((head).notifiers) }
 
 void notifier_list_init(NotifierList *list);
 
 void notifier_list_add(NotifierList *list, Notifier *notifier);
 
-void notifier_list_remove(NotifierList *list, Notifier *notifier);
+void notifier_remove(Notifier *notifier);
 
 void notifier_list_notify(NotifierList *list, void *data);
 
diff --git a/qemu-timer.c b/qemu-timer.c
index cd026c6..2eda9b9 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -453,7 +453,7 @@  void qemu_register_clock_reset_notifier(QEMUClock *clock, Notifier *notifier)
 
 void qemu_unregister_clock_reset_notifier(QEMUClock *clock, Notifier *notifier)
 {
-    notifier_list_remove(&clock->reset_notifiers, notifier);
+    notifier_remove(notifier);
 }
 
 void init_clocks(void)
diff --git a/vl.c b/vl.c
index d925424..a40b134 100644
--- a/vl.c
+++ b/vl.c
@@ -2058,7 +2058,7 @@  void qemu_add_exit_notifier(Notifier *notify)
 
 void qemu_remove_exit_notifier(Notifier *notify)
 {
-    notifier_list_remove(&exit_notifiers, notify);
+    notifier_remove(notify);
 }
 
 static void qemu_run_exit_notifiers(void)