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

Marc-André Lureau marcandre.lureau at gmail.com
Sat Oct 1 23:43:29 UTC 2011


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,
-- 
1.7.6.2




More information about the libvir-list mailing list