[Evolution-hackers] error in generated vcalendar + error reading changed tasks/events

Armin Bauer armin.bauer@desscon.com
Mon, 14 Mar 2005 01:03:06 +0100


This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enigF24D497BD89F9643DACC4FAE
Content-Type: multipart/mixed;
 boundary="------------020108000604010109030907"

This is a multi-part message in MIME format.
--------------020108000604010109030907
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit



JP Rosevear wrote:
> On Wed, 2005-03-09 at 00:09 +0100, Armin Bauer wrote:
>
>>>Anyhow, I think whats really happening is that Evolution wants to use
>>>the CATEGORIES as a whole single string and breaks it up in the UI
>>>itself.  The \, means we are treating the categories as a single block
>>>rather a delimited list, that is valid icalendar, if a bit unusual.
>>>
>>
>>So, judging from your answer, that you are not going to change this?
>
>
> Not in the short term, is there a specific issue this is causing you?

I just have to change your vcard parser to make a exception in this case
then. (btw: i changed your vcard parser to parser vcards, vnotes,
vcalendar and generate these as well)

>>>>>>The second one is that when i add several tasks/events, request them via
>>>>>>e_cal_get_changes, delete all of them and request again using
>>>>>>e_cal_get_changes the returned GList will contain more items than delete
>>>>>>d and the additional items look like random memory locations. I attached
>>>>>>the code that produces this result. I took a look at the bug database
>>>>>>and there are quite some bugs which reporting crashing etc when deleting
>>>>>>tasks/events, so maybe this is related.
>>>>>
>>>>>
>>>>>Probably unrelated, but it might help against some pilot bugs.  What is
>>>>>the exactly version you are coding against?
>>>>
>>>>ii  evolution      2.0.4-0.1      The groupware suite
>>>>ii  evolution-data 1.0.4-0.1      evolution database backend server
>>>>
>>>>entry 20050308T203113Z-6966-1000-6736-0@azrael
>>>>entry 20050308T203113Z-6966-1000-6736-1@azrael
>>>>entry 20050308T203002Z-6781-1000-1-1@azrael
>>>>entry 20050308T203113Z-6966-1000-6736-2@azrael
>>>>entry 20050308T203002Z-6781-1000-1-2@azrael
>>>>entry 20050307T183359Z-32171-1000-1-42@azrael
>>>>
>>>>Note that i deleted 3 tasks (the ones with *1000-1*)
>>>
>>>
>>>Have you looked at the .xml file on disk to see if thats being updated
>>>properly?
>>
>>Yes. i attached the ics and .db files after the first sync (after i
>>added 3 tasks) and after the second one (with the deletion). Nothing
>>suspicious...
>
>
> Well, next step would be to debug the code in the file backend
> (e-d-s/calendar/backends/file).  It will be several days before i could
> get to this, but you can ping me with any questions if you want to take
> a crack.
>

Attached is patch against eds-1.0.4 to fix this bug. The bug was caused
since you modified a hashtable during a call to g_hash_table_foreach.
You call e_xmlhash_foreach_key during
e_cal_backend_file_compute_changes. Then the callback
e_cal_backend_file_compute_changes_foreach_key removes entries. This
works if only 1 is removed but leads to corruption when removing several
items.

See the comments on
http://developer.gnome.org/doc/API/2.0/glib/glib-Hash-Tables.html#g-hash-table-foreach

The correct solution is to provide a new e_xmlhash_foreach_key_remove
function where it is safe to remove a key. The attached patch does this.
I suggest that you check the remaining code for similar errors.

Armin

--------------020108000604010109030907
Content-Type: text/x-patch;
 name="eds-1.0.4.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="eds-1.0.4.patch"

diff -u -r ./calendar/backends/file/e-cal-backend-file.c ../evolution-data-server-1.0.4-new/calendar/backends/file/e-cal-backend-file.c
--- ./calendar/backends/file/e-cal-backend-file.c	2004-12-06 15:28:20.000000000 +0100
+++ ../evolution-data-server-1.0.4-new/calendar/backends/file/e-cal-backend-file.c	2005-03-14 00:50:49.000000000 +0100
@@ -1446,7 +1446,7 @@
 	EXmlHash *ehash;
 } ECalBackendFileComputeChangesData;

-static void
+static gboolean
 e_cal_backend_file_compute_changes_foreach_key (const char *key, gpointer value, gpointer data)
 {
 	ECalBackendFileComputeChangesData *be_data = data;
@@ -1463,8 +1463,9 @@
 		e_cal_component_set_uid (comp, key);
 		be_data->deletes = g_list_prepend (be_data->deletes, e_cal_component_get_as_string (comp));

-		e_xmlhash_remove (be_data->ehash, key);
+		return TRUE;
  	}
+	return FALSE;
 }

 static ECalBackendSyncStatus
@@ -1523,11 +1524,10 @@
 	be_data.kind = e_cal_backend_get_kind (E_CAL_BACKEND (cbfile));
 	be_data.deletes = NULL;
 	be_data.ehash = ehash;
-
-	e_xmlhash_foreach_key (ehash, (EXmlHashFunc)e_cal_backend_file_compute_changes_foreach_key, &be_data);
+
+	e_xmlhash_foreach_key_remove (ehash, (EXmlHashRemoveFunc)e_cal_backend_file_compute_changes_foreach_key, &be_data);

 	*deletes = be_data.deletes;
-
 	e_xmlhash_write (ehash);
   	e_xmlhash_destroy (ehash);

diff -u -r ./libedataserver/e-xml-hash-utils.c ../evolution-data-server-1.0.4-new/libedataserver/e-xml-hash-utils.c
--- ./libedataserver/e-xml-hash-utils.c	2005-02-14 16:26:46.000000000 +0100
+++ ../evolution-data-server-1.0.4-new/libedataserver/e-xml-hash-utils.c	2005-03-14 00:36:50.000000000 +0100
@@ -237,6 +237,32 @@
 	g_hash_table_foreach (hash->objects, foreach_hash_func, &data);
 }

+typedef struct {
+	EXmlHashRemoveFunc func;
+	gpointer user_data;
+} foreach_data_remove_t;
+
+static gboolean
+foreach_hash_remove_func (gpointer  key, gpointer value, gpointer user_data)
+{
+	foreach_data_remove_t *data = (foreach_data_remove_t *) user_data;
+
+	return data->func ((const char *) key, (const char *) value, data->user_data);
+}
+
+void
+e_xmlhash_foreach_key_remove (EXmlHash *hash, EXmlHashRemoveFunc func, gpointer user_data)
+{
+	foreach_data_remove_t data;
+
+	g_return_if_fail (hash != NULL);
+	g_return_if_fail (func != NULL);
+
+	data.func = func;
+	data.user_data = user_data;
+	g_hash_table_foreach_remove (hash->objects, foreach_hash_remove_func, &data);
+}
+
 void
 e_xmlhash_write (EXmlHash *hash)
 {
diff -u -r ./libedataserver/e-xml-hash-utils.h ../evolution-data-server-1.0.4-new/libedataserver/e-xml-hash-utils.h
--- ./libedataserver/e-xml-hash-utils.h	2004-05-03 18:44:56.000000000 +0200
+++ ../evolution-data-server-1.0.4-new/libedataserver/e-xml-hash-utils.h	2005-03-14 00:34:40.000000000 +0100
@@ -47,6 +47,7 @@
 } EXmlHashStatus;

 typedef void (* EXmlHashFunc) (const char *key, const char *value, gpointer user_data);
+typedef gboolean (* EXmlHashRemoveFunc) (const char *key, const char *value, gpointer user_data);

 typedef struct EXmlHash EXmlHash;

@@ -65,6 +66,10 @@
 				      EXmlHashFunc  func,
 				      gpointer      user_data);

+void           e_xmlhash_foreach_key_remove (EXmlHash     *hash,
+				      EXmlHashRemoveFunc  func,
+				      gpointer      user_data);
+
 void           e_xmlhash_write       (EXmlHash     *hash);
 void           e_xmlhash_destroy     (EXmlHash     *hash);


--------------020108000604010109030907--

--------------enigF24D497BD89F9643DACC4FAE
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)

iD8DBQFCNNS9q9z7v9k9UakRAvjwAJ4nK8Pd65IDGAW4fGTHsgcC0KybiQCfdXoT
cgMRtSW/uJTmDhkoiuP1hrk=
=Gos2
-----END PGP SIGNATURE-----

--------------enigF24D497BD89F9643DACC4FAE--