GNOME.org
 

Re: [evolution-patches] Hi,

what the heck happened to your message?

On Fri, 2005-07-22 at 00:55 -0600, Jukaku Mubeen wrote:
> When i add several tasks/events, request them via
> 	
> e_cal_get_changes, delete all of them and
> Mime-Version: 1.0
> Content-Type: multipart/mixed; boundary="=__Part3C1F936E.0__="
> 
> This is a MIME message. If you are reading this text, you may want to 
> consider changing to a mail reader or gateway that understands how to 
> properly handle MIME multipart messages.
> 
> --=__Part3C1F936E.0__=
> Content-Type: multipart/alternative; boundary="=__Part3C1F936E.1__="
> 
> --=__Part3C1F936E.1__=
> Content-Type: text/plain; charset=US-ASCII
> Content-Transfer-Encoding: 7bit
> 
> Hi, 
>  
> 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
> deleted 
> and the additional items look like random memory locations. 
>  
> Attached is patch against e-d-s 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. 
>  
> See
> http://lists.ximian.com/archives/public/evolution-hackers/2005-March/005257.html
> for details. 
>  
> Thanks 
> Mubeen 
> 
> 
> --=__Part3C1F936E.1__=
> Content-Type: text/html; charset=ISO-8859-1
> Content-Transfer-Encoding: quoted-printable
> 
> <html>
>   <head>
>     <style type=3D"text/css">
>       <!--
>         body { margin-right: 4px; margin-bottom: 1px; margin-top: 4px; =
> margin-left: 4px; font-variant: normal; line-height: normal }
>       -->
>     </style>
>    =20
>   </head>
>   <body>
>     <DIV>
>       Hi&#44;
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       When i add several tasks/events&#44; request them via=20
>     </DIV>
>     <DIV>
>       e_cal_get_changes&#44; delete all of them and request again using=20
>     </DIV>
>     <DIV>
>       e_cal_get_changes the returned GList will contain more items than =
> deleted
>     </DIV>
>     <DIV>
>       and the additional items look like random memory locations.
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       Attached is patch against e-d-s to fix this bug.
>     </DIV>
>     <DIV>
>       The bug was caused since you modified a hashtable during a call to =
> g_hash_table_foreach.
>     </DIV>
>     <DIV>
>       You call e_xmlhash_foreach_key during
>     </DIV>
>     <DIV>
>       e_cal_backend_file_compute_changes. Then the callback
>     </DIV>
>     <DIV>
>       e_cal_backend_file_compute_changes_foreach_key removes entries. This
>     </DIV>
>     <DIV>
>       works if only 1 is removed but leads to corruption when removing =
> several
>     </DIV>
>     <DIV>
>       items.
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       See the comments on
>     </DIV>
>     <DIV>
>       http://developer.gnome.org/doc/API/2.0/glib/glib-Hash-Tables.html#=
> ;g-hash-table-foreach
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       The correct solution is to provide a new e_xmlhash_foreach_key_remove=
> 
>     </DIV>
>     <DIV>
>       function where it is safe to remove a key. The attached patch does =
> this.
>     </DIV>
>     <DIV>
>       I suggest that you check the remaining code for similar errors.
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       See <font color=3D"#0000ff"><a href=3D"http://lists.ximian.com/archiv=
> es/public/evolution-hackers/2005-March/005257.html"><u><i>http://lists.ximi=
> an.com/archives/public/evolution-hackers/2005-March/005257.html</i></u></a>=
> </font>&nbsp;for details.
>     </DIV>
>     <DIV>&nbsp;</DIV>
>     <DIV>
>       Thanks
>     </DIV>
>     <DIV>
>       Mubeen
>     </DIV>
>   </body>
> </html>
> 
> --=__Part3C1F936E.1__=--
> 
> --=__Part3C1F936E.0__=
> Content-Type: text/plain; name="eds.patch"
> Content-Transfer-Encoding: 8bit
> Content-Disposition: attachment; filename="eds.patch"
> 
> --- calendar/backends/file/e-cal-backend-file.c	2005-07-21 18:24:47.255439795 +0530
> +++ ../evolution-data-server.new/calendar/backends/file/e-cal-backend-file.c	2005-07-21 18:25:28.372232186 +0530
> @@ -1477,7 +1477,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;
> @@ -1494,8 +1494,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
> @@ -1555,7 +1556,7 @@
>  	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;
>  
> --- libedataserver/e-xml-hash-utils.c	2005-07-21 18:24:47.256439669 +0530
> +++ ../evolution-data-server.new/libedataserver/e-xml-hash-utils.c	2005-07-20 17:03:08.000000000 +0530
> @@ -319,6 +319,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);
> +}
> +
>  /**
>   * e_xmlhash_write:
>   * @hash: The #EXmlHash to write.
> --- libedataserver/e-xml-hash-utils.h	2005-07-21 18:24:47.257439542 +0530
> +++ ../evolution-data-server.new/libedataserver/e-xml-hash-utils.h	2005-07-21 18:31:04.460638126 +0530
> @@ -58,6 +58,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;
>  
> @@ -76,6 +77,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);
>  
> 
> --=__Part3C1F936E.0__=--
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches
> 
-- 
Jeffrey Stedfast
Evolution Hacker - Novell, Inc.
fejj ximian com  - www.novell.com
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]