Message ID | 1325527237-24146-2-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
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
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
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
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)
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(-)