Re: [libvirt] [libvirt-glib] event: remove timeout and handle from arrays

Otherwise, it will crash next time it goes find(). ==9856== Invalid read of size 4 ==9856== at 0x84E4888: gvir_event_timeout_find (libvirt-glib-event.c:293) ==9856== by 0x84E48E4: gvir_event_timeout_update (libvirt-glib-event.c:308) ==9856== by 0x31AB04BBDB: virEventUpdateTimeout (event.c:122) ==9856== by 0x31AB148758: virNetClientStreamEventTimerUpdate (virnetclientstream.c:81) ==9856== by 0x31AB14991C: virNetClientStreamEventAddCallback (virnetclientstream.c:471) ==9856== by 0x31AB12944F: remoteStreamEventAddCallback (remote_driver.c:3484) ==9856== by 0x31AB108EAC: virStreamEventAddCallback (libvirt.c:14036) ... ==9856== Address 0x143be760 is 0 bytes inside a block of size 40 free'd ==9856== at 0x4A06928: free (vg_replace_malloc.c:427) ==9856== by 0x84E4AD6: gvir_event_timeout_remove (libvirt-glib-event.c:361) ==9856== by 0x31AB04BC2C: virEventRemoveTimeout (event.c:136) ==9856== by 0x31AB149B24: virNetClientStreamEventRemoveCallback (virnetclientstream.c:521) ==9856== by 0x31AB129566: remoteStreamEventRemoveCallback (remote_driver.c:3525) ==9856== by 0x31AB10918F: virStreamEventRemoveCallback (libvirt.c:14114) ... --- libvirt-glib/libvirt-glib-event.c | 83 +++++++++++++++++++++++------------- 1 files changed, 53 insertions(+), 30 deletions(-) diff --git a/libvirt-glib/libvirt-glib-event.c b/libvirt-glib/libvirt-glib-event.c index 8b1bddf..303bec7 100644 --- a/libvirt-glib/libvirt-glib-event.c +++ b/libvirt-glib/libvirt-glib-event.c @@ -61,12 +61,10 @@ struct gvir_event_timeout GMutex *eventlock = NULL; static int nextwatch = 1; -static unsigned int nhandles = 0; -static struct gvir_event_handle **handles = NULL; +static GPtrArray *handles; static int nexttimer = 1; -static unsigned int ntimeouts = 0; -static struct gvir_event_timeout **timeouts = NULL; +static GPtrArray *timeouts; static gboolean gvir_event_handle_dispatch(GIOChannel *source G_GNUC_UNUSED, @@ -106,9 +104,7 @@ gvir_event_handle_add(int fd, g_mutex_lock(eventlock); - handles = g_realloc(handles, sizeof(*handles)*(nhandles+1)); - data = g_malloc(sizeof(*data)); - memset(data, 0, sizeof(*data)); + data = g_new0(struct gvir_event_handle, 1); if (events & VIR_EVENT_HANDLE_READABLE) cond |= G_IO_IN; @@ -130,7 +126,7 @@ gvir_event_handle_add(int fd, gvir_event_handle_dispatch, data); - handles[nhandles++] = data; + g_ptr_array_add(handles, data); ret = data->watch; @@ -140,12 +136,24 @@ gvir_event_handle_add(int fd, } static struct gvir_event_handle * -gvir_event_handle_find(int watch) +gvir_event_handle_find(int watch, guint *idx) { - unsigned int i; - for (i = 0 ; i < nhandles ; i++) - if (handles[i]->watch == watch) - return handles[i]; + guint i; + + for (i = 0 ; i < handles->len ; i++) { + struct gvir_event_handle *h = g_ptr_array_index(handles, i); + + if (h == NULL) { + g_warn_if_reached (); + continue; + } + + if (h->watch == watch) { + if (idx != NULL) + *idx = i; + return h; + } + } return NULL; } @@ -158,7 +166,7 @@ gvir_event_handle_update(int watch, g_mutex_lock(eventlock); - data = gvir_event_handle_find(watch); + data = gvir_event_handle_find(watch, NULL); if (!data) { DEBUG("Update for missing handle watch %d", watch); goto cleanup; @@ -202,7 +210,8 @@ _handle_remove(gpointer data) if (h->ff) (h->ff)(h->opaque); - free(h); + + g_ptr_array_remove_fast(handles, h); return FALSE; } @@ -212,10 +221,11 @@ gvir_event_handle_remove(int watch) { struct gvir_event_handle *data; int ret = -1; + guint idx; g_mutex_lock(eventlock); - data = gvir_event_handle_find(watch); + data = gvir_event_handle_find(watch, &idx); if (!data) { DEBUG("Remove of missing watch %d", watch); goto cleanup; @@ -259,10 +269,7 @@ gvir_event_timeout_add(int interval, g_mutex_lock(eventlock); - timeouts = g_realloc(timeouts, sizeof(*timeouts)*(ntimeouts+1)); - data = g_malloc(sizeof(*data)); - memset(data, 0, sizeof(*data)); - + data = g_new0(struct gvir_event_timeout, 1); data->timer = nexttimer++; data->interval = interval; data->cb = cb; @@ -273,7 +280,7 @@ gvir_event_timeout_add(int interval, gvir_event_timeout_dispatch, data); - timeouts[ntimeouts++] = data; + g_ptr_array_add(timeouts, data); DEBUG("Add timeout %p %d %p %p %d\n", data, interval, cb, opaque, data->timer); @@ -286,12 +293,26 @@ gvir_event_timeout_add(int interval, static struct gvir_event_timeout * -gvir_event_timeout_find(int timer) +gvir_event_timeout_find(int timer, guint *idx) { - unsigned int i; - for (i = 0 ; i < ntimeouts ; i++) - if (timeouts[i]->timer == timer) - return timeouts[i]; + guint i; + + g_return_val_if_fail(timeouts != NULL, NULL); + + for (i = 0 ; i < timeouts->len ; i++) { + struct gvir_event_timeout *t = g_ptr_array_index(timeouts, i); + + if (t == NULL) { + g_warn_if_reached (); + continue; + } + + if (t->timer == timer) { + if (idx != NULL) + *idx = i; + return t; + } + } return NULL; } @@ -305,7 +326,7 @@ gvir_event_timeout_update(int timer, g_mutex_lock(eventlock); - data = gvir_event_timeout_find(timer); + data = gvir_event_timeout_find(timer, NULL); if (!data) { DEBUG("Update of missing timer %d", timer); goto cleanup; @@ -337,11 +358,12 @@ static int gvir_event_timeout_remove(int timer) { struct gvir_event_timeout *data; + guint idx; int ret = -1; g_mutex_lock(eventlock); - data = gvir_event_timeout_find(timer); + data = gvir_event_timeout_find(timer, &idx); if (!data) { DEBUG("Remove of missing timer %d", timer); goto cleanup; @@ -358,8 +380,7 @@ gvir_event_timeout_remove(int timer) if (data->ff) (data->ff)(data->opaque); - free(data); - + g_ptr_array_remove_index_fast(timeouts, idx); ret = 0; cleanup: @@ -371,6 +392,8 @@ cleanup: static gpointer event_register_once(gpointer data G_GNUC_UNUSED) { eventlock = g_mutex_new(); + timeouts = g_ptr_array_new_with_free_func(g_free); + handles = g_ptr_array_new_with_free_func(g_free); virEventRegisterImpl(gvir_event_handle_add, gvir_event_handle_update, gvir_event_handle_remove,