Reference state is not necessary when virEventAddTimeout failed,
this may cause a memory leak, so reference state only when
virEventAddTimeout success.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com>
---
src/conf/object_event.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index 5d84598d59..ee5def5910 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -891,20 +891,21 @@ virObjectEventStateRegisterID(virConnectPtr conn,
virObjectLock(state);
if ((state->callbacks->count == 0) &&
- (state->timer == -1) &&
- (state->timer = virEventAddTimeout(-1,
- virObjectEventTimer,
- state,
- virObjectFreeCallback)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not initialize domain event timer"));
- goto cleanup;
+ (state->timer == -1)) {
+ if ((state->timer = virEventAddTimeout(-1,
+ virObjectEventTimer,
+ state,
+ virObjectFreeCallback)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not initialize domain event timer"));
+ goto cleanup;
+ } else {
+ /* event loop has one reference, but we need one more for the
+ * timer's opaque argument */
+ virObjectRef(state);
+ }
}
- /* event loop has one reference, but we need one more for the
- * timer's opaque argument */
- virObjectRef(state);
-
ret = virObjectEventCallbackListAddID(conn, state->callbacks,
key, filter, filter_opaque,
klass, eventID,
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 9/19/19 5:02 AM, Xu Yandong wrote: > Reference state is not necessary when virEventAddTimeout failed, > this may cause a memory leak, so reference state only when > virEventAddTimeout success. > > Signed-off-by: Xu Yandong <xuyandong2@huawei.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > src/conf/object_event.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/src/conf/object_event.c b/src/conf/object_event.c > index 5d84598d59..ee5def5910 100644 > --- a/src/conf/object_event.c > +++ b/src/conf/object_event.c > @@ -891,20 +891,21 @@ virObjectEventStateRegisterID(virConnectPtr conn, > virObjectLock(state); > > if ((state->callbacks->count == 0) && > - (state->timer == -1) && > - (state->timer = virEventAddTimeout(-1, > - virObjectEventTimer, > - state, > - virObjectFreeCallback)) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("could not initialize domain event timer")); > - goto cleanup; > + (state->timer == -1)) { > + if ((state->timer = virEventAddTimeout(-1, > + virObjectEventTimer, > + state, > + virObjectFreeCallback)) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("could not initialize domain event timer")); > + goto cleanup; > + } else { > + /* event loop has one reference, but we need one more for the > + * timer's opaque argument */ > + virObjectRef(state); > + } > } > > - /* event loop has one reference, but we need one more for the > - * timer's opaque argument */ > - virObjectRef(state); > - > ret = virObjectEventCallbackListAddID(conn, state->callbacks, > key, filter, filter_opaque, > klass, eventID, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 9/19/19 10:02 AM, Xu Yandong wrote: > Reference state is not necessary when virEventAddTimeout failed, > this may cause a memory leak, so reference state only when > virEventAddTimeout success. > > Signed-off-by: Xu Yandong <xuyandong2@huawei.com> > --- > src/conf/object_event.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/src/conf/object_event.c b/src/conf/object_event.c > index 5d84598d59..ee5def5910 100644 > --- a/src/conf/object_event.c > +++ b/src/conf/object_event.c > @@ -891,20 +891,21 @@ virObjectEventStateRegisterID(virConnectPtr conn, > virObjectLock(state); > > if ((state->callbacks->count == 0) && > - (state->timer == -1) && > - (state->timer = virEventAddTimeout(-1, > - virObjectEventTimer, > - state, > - virObjectFreeCallback)) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("could not initialize domain event timer")); > - goto cleanup; > + (state->timer == -1)) { > + if ((state->timer = virEventAddTimeout(-1, > + virObjectEventTimer, > + state, > + virObjectFreeCallback)) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("could not initialize domain event timer")); > + goto cleanup; > + } else { > + /* event loop has one reference, but we need one more for the > + * timer's opaque argument */ > + virObjectRef(state); > + } This doesn't have to be under else branch since there's no way the control can return from the above branch. > } > > - /* event loop has one reference, but we need one more for the > - * timer's opaque argument */ > - virObjectRef(state); > - > ret = virObjectEventCallbackListAddID(conn, state->callbacks, > key, filter, filter_opaque, > klass, eventID, > I'm rewording the commit message a bit, ACKing and pushing. Congratulations on your first libvirt contribution. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.