GNOME.org
 

Re: gnome-pilot Evolution conduits ported to pilot-link 0.12pre4

> I've ported the Evolution 2.6 (and 2.7 - they're the same) conduits to pilot-
> link 0.12pre4. They
> are also backwards-compatible with pilot-link 0.11.8 (used the same mechanism
>  that Matt put into
> gnome-pilot 2.0.14preX).

for the record, credit for the PILOT_LINK_0_12 defines in gnome-pilot
goes to jpr.

> I'm planning on submitting them to the evolution developers soon so
> they'll make it into Evolution 2.8.

I don't have time right now to do a full review, but I spotted what may
be an issue in the first patch section (may be a pattern).


> Index: address-conduit.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/addressbook/conduit/address-conduit.c,v
> retrieving revision 1.88
> diff -u -r1.88 address-conduit.c
> --- address-conduit.c	6 Dec 2005 08:43:37 -0000	1.88
> +++ address-conduit.c	20 Aug 2006 19:50:34 -0000
[....]
>  static GnomePilotRecord
>  local_record_to_pilot_record (EAddrLocalRecord *local,
> -			      EAddrConduitContext *ctxt)
> +			      EAddrConduitContext *ctxt,
> +			      unsigned char *record,
> +			      int maxRecordLen)
>  {
>  	GnomePilotRecord p;
> -	static char record[0xffff];
>  	
>  	g_assert (local->addr != NULL );
>  	
> @@ -804,8 +825,19 @@
>  	p.secret = local->local.secret;
>  
>  	/* Generate pilot record structure */
> +#ifdef PILOT_LINK_0_12
> +	{
> +		pi_buffer_t piBuf;
> +		piBuf.data = record;
> +		piBuf.allocated = maxRecordLen;
> +		piBuf.used = 0;
> +		p.length = pack_Address(local->addr, &piBuf, address_v1);
> +		p.record = piBuf.data;
> +	}
> +#else
>  	p.record = record;
> -	p.length = pack_Address (local->addr, p.record, 0xffff);
> +	p.length = pack_Address (local->addr, p.record, maxRecordLen);
> +#endif


In the above code, the user supplies *record.  This memory is then
assigned to piBuf.data, and may therefore be freed by pack_Address
if it needs to reallocate the pi_buffer_t.  So, if the user passes
in a pointer to a static array we could be in trouble.

One of the good things (sorry, David!) to come out of fedora's packaging
of 0.12.0pre was that the evo conduits got ported to the 0.12.0 API
as a fedora patch.  They didn't make it backwards compatible with 0.11.8,
but it would be worth comparing your patch to the fedora patch (as we'll
probably find issues in both that way!).  I'll append the patch here.

Would be great to submit a patch to evolution.  I'd cc jpr and fejj.

All the best,

Matt

--- evolution-2.5.4/addressbook/conduit/address-conduit.c.fix-conduits	2005-12-08 03:15:02.000000000 -0500
+++ evolution-2.5.4/addressbook/conduit/address-conduit.c	2006-01-10 19:33:44.000000000 -0500
@@ -462,14 +462,19 @@
 {
 	static char buff[ 4096 ];
 	struct Address addr;
+	pi_buffer_t piBuf;
 
 	if (remote == NULL) {
 		sprintf (buff, "[NULL]");
 		return buff;
 	}
 
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&addr, 0, sizeof (struct Address));
-	unpack_Address (&addr, remote->record, remote->length);
+	unpack_Address (&addr, &piBuf, address_v1);
 
 	g_snprintf (buff, 4096, "['%s' '%s' '%s']",
 		    addr.entry[entryLastname] ?
@@ -791,7 +796,8 @@
 			      EAddrConduitContext *ctxt)
 {
 	GnomePilotRecord p;
-	static char record[0xffff];
+	static unsigned char record[0xffff];
+	pi_buffer_t piBuf;
 	
 	g_assert (local->addr != NULL );
 	
@@ -803,9 +809,17 @@
 	p.archived = local->local.archived;
 	p.secret = local->local.secret;
 
+	memset (&piBuf, 0, sizeof (piBuf));
+	memset (record, 0, sizeof (record));
+	pack_Address (local->addr, &piBuf, address_v1);
+
 	/* Generate pilot record structure */
+	if (piBuf.used > 0)
+		memcpy (record, piBuf.data, piBuf.used);
 	p.record = record;
-	p.length = pack_Address (local->addr, p.record, 0xffff);
+	p.length = piBuf.used;
+	if (piBuf.data)
+		free (piBuf.data);
 
 	return p;	
 }
@@ -834,16 +848,16 @@
 	 */
 	if (local->local.ID != 0) {
 		struct Address addr;
-		char record[0xffff];
+		pi_buffer_t *buffer = pi_buffer_new (0xffff);
 		int cat = 0;
 		
 		if (dlp_ReadRecordById (ctxt->dbi->pilot_socket, 
 					ctxt->dbi->db_handle,
-					local->local.ID, &record, 
-					NULL, NULL, NULL, &cat) > 0) {
+					local->local.ID, buffer, 
+					NULL, NULL, &cat) > 0) {
 			local->local.category = cat;
 			memset (&addr, 0, sizeof (struct Address));
-			unpack_Address (&addr, record, 0xffff);
+			unpack_Address (&addr, buffer, address_v1);
 			for (i = 0; i < 5; i++) {
 				if (addr.entry[entryPhone1 + i])
 					local->addr->entry[entryPhone1 + i] = 
@@ -858,6 +872,8 @@
 			}
 			free_Address (&addr);
 		}
+
+		pi_buffer_free (buffer);
 	}
 
 	local->addr->entry[entryFirstname] = e_pilot_utf8_to_pchar (e_contact_get_const (contact, E_CONTACT_GIVEN_NAME));
@@ -1019,10 +1035,16 @@
 	EContactField next_mail, next_home, next_work, next_fax;
 	EContactField next_other, next_main, next_pager, next_mobile;
 	int i;
+	pi_buffer_t piBuf;
 
 	g_return_val_if_fail(remote!=NULL,NULL);
 	memset (&address, 0, sizeof (struct Address));
-	unpack_Address (&address, remote->record, remote->length);
+
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
+	unpack_Address (&address, &piBuf, address_v1);
 
 	if (in_contact == NULL)
 		contact = e_contact_new ();
@@ -1212,7 +1234,7 @@
 	EBookQuery *query;
     	GList *l;
 	int len;
-	unsigned char *buf;
+	pi_buffer_t *buffer;
 	char *filename;
 	char *change_id;
 	char *auth;
@@ -1302,9 +1324,9 @@
   	gnome_pilot_conduit_sync_abs_set_num_updated_local_records (abs_conduit, mod_records);
   	gnome_pilot_conduit_sync_abs_set_num_deleted_local_records(abs_conduit, del_records);
 
-	buf = (unsigned char*)g_malloc (0xffff);
+	buffer = pi_buffer_new (0xffff);
 	len = dlp_ReadAppBlock (dbi->pilot_socket, dbi->db_handle, 0,
-			      (unsigned char *)buf, 0xffff);
+				-1, buffer);
 	
 	if (len < 0) {
 		WARN (_("Could not read pilot's Address application block"));
@@ -1313,8 +1335,8 @@
 					   _("Could not read pilot's Address application block"));
 		return -1;
 	}
-	unpack_AddressAppInfo (&(ctxt->ai), buf, len);
-	g_free (buf);
+	unpack_AddressAppInfo (&(ctxt->ai), buffer->data, len);
+	pi_buffer_free (buffer);
 
   	check_for_slow_setting (conduit, ctxt);
 	if (ctxt->cfg->sync_type == GnomePilotConduitSyncTypeCopyToPilot
--- evolution-2.5.4/calendar/conduits/calendar/calendar-conduit.c.fix-conduits	2006-01-02 06:38:57.000000000 -0500
+++ evolution-2.5.4/calendar/conduits/calendar/calendar-conduit.c	2006-01-10 19:33:44.000000000 -0500
@@ -413,14 +413,20 @@
 {
 	static char buff[ 4096 ];
 	struct Appointment appt;
+	pi_buffer_t piBuf;
 
 	if (remote == NULL) {
 		sprintf (buff, "[NULL]");
 		return buff;
 	}
 
+
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&appt, 0, sizeof (struct Appointment));
-	unpack_Appointment (&appt, remote->record, remote->length);
+	unpack_Appointment (&appt, &piBuf, datebook_v1);
 
 	g_snprintf (buff, 4096, "[%ld %ld '%s' '%s']",
 		    mktime (&appt.begin),
@@ -818,7 +824,8 @@
 			      ECalConduitContext *ctxt)
 {
 	GnomePilotRecord p;
-	static char record[0xffff];
+	static unsigned char record[0xffff];
+	pi_buffer_t piBuf;
 
 	g_assert (local->comp != NULL);
 	g_assert (local->appt != NULL );
@@ -829,9 +836,17 @@
 	p.archived = local->local.archived;
 	p.secret = local->local.secret;
 
+	memset (&piBuf, 0, sizeof (piBuf));
+	memset (record, 0, sizeof (record));
+	pack_Appointment (local->appt, &piBuf, datebook_v1);
+
 	/* Generate pilot record structure */
+	if (piBuf.used > 0)
+		memcpy (record, piBuf.data, piBuf.used);
 	p.record = record;
-	p.length = pack_Appointment (local->appt, p.record, 0xffff);
+	p.length = piBuf.used;
+	if (piBuf.data)
+		free (piBuf.data);
 
 	return p;	
 }
@@ -867,22 +882,24 @@
          * we don't overwrite them 
 	 */
 	if (local->local.ID != 0) {
-		struct Appointment appt;		
-		char record[0xffff];
+		struct Appointment appt;
+		pi_buffer_t *buffer = pi_buffer_new (0xffff);
 		int cat = 0;
 		
 		if (dlp_ReadRecordById (ctxt->dbi->pilot_socket, 
 					ctxt->dbi->db_handle,
-					local->local.ID, &record, 
-					NULL, NULL, NULL, &cat) > 0) {
+					local->local.ID, buffer,
+					NULL, NULL, &cat) > 0) {
 			local->local.category = cat;
 			memset (&appt, 0, sizeof (struct Appointment));
-			unpack_Appointment (&appt, record, 0xffff);
+			unpack_Appointment (&appt, buffer, datebook_v1);
 			local->appt->alarm = appt.alarm;
 			local->appt->advance = appt.advance;
 			local->appt->advanceUnits = appt.advanceUnits;
 			free_Appointment (&appt);
 		}
+
+		pi_buffer_free (buffer);
 	}
 
 	/* STOP: don't replace these with g_strdup, since free_Appointment
@@ -1140,11 +1157,17 @@
 	GSList *edl = NULL;	
 	char *txt;
 	int pos, i;
+	pi_buffer_t piBuf;
 	
 	g_return_val_if_fail (remote != NULL, NULL);
 
+
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&appt, 0, sizeof (struct Appointment));
-	unpack_Appointment (&appt, remote->record, remote->length);
+	unpack_Appointment (&appt, &piBuf, datebook_v1);
 
 	if (in_comp == NULL) {
 		comp = e_cal_component_new ();
@@ -1409,7 +1432,7 @@
 	GnomePilotConduitSyncAbs *abs_conduit;
 	GList *removed = NULL, *added = NULL, *l;
 	int len;
-	unsigned char *buf;
+	pi_buffer_t *buffer;
 	char *filename, *change_id;
 	icalcomponent *icalcomp;
 	gint num_records, add_records = 0, mod_records = 0, del_records = 0;
@@ -1521,9 +1544,9 @@
 	gnome_pilot_conduit_sync_abs_set_num_updated_local_records (abs_conduit, mod_records);
 	gnome_pilot_conduit_sync_abs_set_num_deleted_local_records(abs_conduit, del_records);
 
-	buf = (unsigned char*)g_malloc (0xffff);
+	buffer = pi_buffer_new (0xffff);
 	len = dlp_ReadAppBlock (dbi->pilot_socket, dbi->db_handle, 0,
-			      (unsigned char *)buf, 0xffff);
+				-1, buffer);
 	
 	if (len < 0) {
 		WARN (_("Could not read pilot's Calendar application block"));
@@ -1532,8 +1555,8 @@
 					   _("Could not read pilot's Calendar application block"));
 		return -1;
 	}
-	unpack_AppointmentAppInfo (&(ctxt->ai), buf, len);
-	g_free (buf);
+	unpack_AppointmentAppInfo (&(ctxt->ai), buffer->data, len);
+	pi_buffer_free (buffer);
 
 	check_for_slow_setting (conduit, ctxt);
 	if (ctxt->cfg->sync_type == GnomePilotConduitSyncTypeCopyToPilot
--- evolution-2.5.4/calendar/conduits/todo/todo-conduit.c.fix-conduits	2005-12-08 03:15:03.000000000 -0500
+++ evolution-2.5.4/calendar/conduits/todo/todo-conduit.c	2006-01-10 19:33:44.000000000 -0500
@@ -402,14 +402,19 @@
 {
 	static char buff[ 4096 ];
 	struct ToDo todo;
+	pi_buffer_t piBuf;
 
 	if (remote == NULL) {
 		sprintf (buff, "[NULL]");
 		return buff;
 	}
 
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&todo, 0, sizeof (struct ToDo));
-	unpack_ToDo (&todo, remote->record, remote->length);
+	unpack_ToDo (&todo, &piBuf, todo_v1);
 
 	g_snprintf (buff, 4096, "[%d %ld %d %d '%s' '%s' %d]",
 		    todo.indefinite,
@@ -594,7 +599,8 @@
 			      EToDoConduitContext *ctxt)
 {
 	GnomePilotRecord p;
-	static char record[0xffff];
+	static unsigned char record[0xffff];
+	pi_buffer_t piBuf;
 
 	g_assert (local->comp != NULL);
 	g_assert (local->todo != NULL );
@@ -607,9 +613,17 @@
 	p.archived = local->local.archived;
 	p.secret = local->local.secret;
 
+	memset (&piBuf, 0, sizeof (piBuf));
+	memset (record, 0, sizeof (record));
+	pack_ToDo (local->todo, &piBuf, todo_v1);
+
 	/* Generate pilot record structure */
+	if (piBuf.used > 0)
+		memcpy (record, piBuf.data, piBuf.used);
 	p.record = record;
-	p.length = pack_ToDo (local->todo, p.record, 0xffff);
+	p.length = piBuf.used;
+	if (piBuf.data)
+		free (piBuf.data);
 
 	return p;	
 }
@@ -696,15 +710,17 @@
 
 	/* Don't overwrite the category */
 	if (local->local.ID != 0) {
-		char record[0xffff];
+		pi_buffer_t *buffer = pi_buffer_new (0xffff);  		
 		int cat = 0;
 		
 		if (dlp_ReadRecordById (ctxt->dbi->pilot_socket, 
 					ctxt->dbi->db_handle,
-					local->local.ID, &record, 
-					NULL, NULL, NULL, &cat) > 0) {
+					local->local.ID, buffer, 
+					NULL, NULL, &cat) > 0) {
 			local->local.category = cat;
 		}
+
+		pi_buffer_free (buffer);
 	}
 	
 	/*
@@ -859,12 +875,17 @@
 	icaltimezone *utc_zone;
 	int priority;
 	char *txt;
+	pi_buffer_t piBuf;
 	char *category;
 	
 	g_return_val_if_fail (remote != NULL, NULL);
 
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&todo, 0, sizeof (struct ToDo));
-	unpack_ToDo (&todo, remote->record, remote->length);
+	unpack_ToDo (&todo, &piBuf, todo_v1);
 
 	utc_zone = icaltimezone_get_utc_timezone ();
 	now = icaltime_from_timet_with_zone (time (NULL), FALSE, 
@@ -1014,7 +1035,7 @@
 	GnomePilotConduitSyncAbs *abs_conduit;
 	GList *l;
 	int len;
-	unsigned char *buf;
+	pi_buffer_t *buffer;
 	char *filename, *change_id;
 	icalcomponent *icalcomp;
 	gint num_records, add_records = 0, mod_records = 0, del_records = 0;
@@ -1104,9 +1125,9 @@
 	g_message("num_records: %d\nadd_records: %d\nmod_records: %d\ndel_records: %d\n",
 			num_records, add_records, mod_records, del_records);
 
-	buf = (unsigned char*)g_malloc (0xffff);
+	buffer = pi_buffer_new (0xffff);
 	len = dlp_ReadAppBlock (dbi->pilot_socket, dbi->db_handle, 0,
-			      (unsigned char *)buf, 0xffff);
+				-1, buffer);
 	
 	if (len < 0) {
 		WARN (_("Could not read pilot's ToDo application block"));
@@ -1115,8 +1136,8 @@
 					   _("Could not read pilot's ToDo application block"));
 		return -1;
 	}
-	unpack_ToDoAppInfo (&(ctxt->ai), buf, len);
-	g_free (buf);
+	unpack_ToDoAppInfo (&(ctxt->ai), buffer->data, len);
+	pi_buffer_free (buffer);
 	
 	lastDesktopUniqueID = 128;
 
--- evolution-2.5.4/calendar/conduits/memo/memo-conduit.c.fix-conduits	2006-01-10 22:52:28.000000000 -0500
+++ evolution-2.5.4/calendar/conduits/memo/memo-conduit.c	2006-01-10 23:11:47.000000000 -0500
@@ -331,14 +331,19 @@
 {
 	static char buff[ 64 ];
 	struct Memo memo;
+	pi_buffer_t piBuf;
 
 	if (remote == NULL) {
 		sprintf (buff, "[NULL]");
 		return buff;
 	}
 
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&memo, 0, sizeof (struct Memo));
-	unpack_Memo (&memo, remote->record, remote->length);
+	unpack_Memo (&memo, &piBuf, memo_v1);
 
 	g_snprintf (buff, 64, "['%s']",
 		    memo.text ?
@@ -451,7 +456,8 @@
 			      EMemoConduitContext *ctxt)
 {
 	GnomePilotRecord p;
-	static char record[0xffff];
+	static unsigned char record[0xffff];
+	pi_buffer_t piBuf;  		
 
 	g_assert (local->comp != NULL);
 	g_assert (local->memo != NULL );
@@ -466,8 +472,14 @@
 
 	/* Generate pilot record structure */
 	p.record = record;
-	p.length = pack_Memo (local->memo, p.record, 0xffff);
-
+	memset (&piBuf, 0, sizeof (piBuf));
+	memset (record, 0, sizeof (record));
+	p.length = pack_Memo (local->memo, &piBuf, memo_v1);
+	if (piBuf.used > 0)
+		memcpy (record, piBuf.data, piBuf.used);
+	p.length = piBuf.used;
+	if (piBuf.data)
+		free (piBuf.data);
 	return p;	
 }
 
@@ -568,16 +580,17 @@
 
 	/* Don't overwrite the category */
 	if (local->local.ID != 0) {
-		char record[0xffff];
+		pi_buffer_t *buffer = pi_buffer_new (0xffff);
 		int cat = 0;
 		
 		LOG(fprintf(stderr, "local_record_from_comp: calling dlp_ReadRecordById\n"));
 		if (dlp_ReadRecordById (ctxt->dbi->pilot_socket, 
 					ctxt->dbi->db_handle,
-					local->local.ID, &record, 
-					NULL, NULL, NULL, &cat) > 0) {
+					local->local.ID, buffer, 
+					NULL, NULL, &cat) > 0) {
 			local->local.category = cat;
 		}
+		pi_buffer_free (buffer);
 		LOG(fprintf(stderr, "local_record_from_comp: done calling dlp_ReadRecordById\n"));
 	}
 	
@@ -699,6 +712,7 @@
 {
 	ECalComponent *comp;
 	struct Memo memo;
+	pi_buffer_t piBuf;
 	struct icaltimetype now;
 	icaltimezone *utc_zone;
 	char *txt, *txt2, *txt3;
@@ -707,8 +721,12 @@
 	
 	g_return_val_if_fail (remote != NULL, NULL);
 
+	piBuf.data = remote->record;
+	piBuf.allocated = remote->length;
+	piBuf.used = remote->length;
+
 	memset (&memo, 0, sizeof (struct Memo));
-	unpack_Memo (&memo, remote->record, remote->length);
+	unpack_Memo (&memo, &piBuf, memo_v1);
 
 	utc_zone = icaltimezone_get_utc_timezone ();
 	now = icaltime_from_timet_with_zone (time (NULL), FALSE, 
@@ -836,7 +854,7 @@
 	GnomePilotConduitSyncAbs *abs_conduit;
 	GList *l;
 	int len;
-	unsigned char *buf;
+	pi_buffer_t *buffer;
 	char *filename, *change_id;
 	icalcomponent *icalcomp;
 	gint num_records, add_records = 0, mod_records = 0, del_records = 0;
@@ -929,9 +947,9 @@
 	g_message("num_records: %d\nadd_records: %d\nmod_records: %d\ndel_records: %d\n",
 		num_records, add_records, mod_records, del_records);
 
-	buf = (unsigned char*)g_malloc (0xffff);
+	buffer = pi_buffer_new (0xffff);
 	len = dlp_ReadAppBlock (dbi->pilot_socket, dbi->db_handle, 0,
-			      (unsigned char *)buf, 0xffff);
+				-1, buffer);
 	
 	if (len < 0) {
 		WARN (_("Could not read pilot's Memo application block"));
@@ -940,8 +958,8 @@
 					   _("Could not read pilot's Memo application block"));
 		return -1;
 	}
-	unpack_MemoAppInfo (&(ctxt->ai), buf, len);
-	g_free (buf);
+	unpack_MemoAppInfo (&(ctxt->ai), buffer->data, len);
+	pi_buffer_free (buffer);
 	
 	lastDesktopUniqueID = 128;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]