[libvirt] [libvirt-glib] event: remove timeout and handle from arrays
Daniel P. Berrange
berrange at redhat.com
Mon Oct 3 08:40:44 UTC 2011
On Sun, Oct 02, 2011 at 01:43:29AM +0200, Marc-André Lureau wrote:
> 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,
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list