Menu
▾
▴
opensync-devel
|
From: Daniel G. <go...@b1...> - 2009-09-17 00:55:33
|
Hi, could you all double-check the public interfaces of the OpenSync trunk API, if they fit your needs for you application/plugin ... (those which got exported by the OSYNC_EXPORT macro) I plan to _soft_ freeze thursday evening/night (CEST) the OpenSync API and do a quick accepteance test with various "hot-requested" plugins ... and port those which aren't yet ported. If you need assistance porting - please drop a mail on opensync-devel mailing list. By "soft freeze" i plan to accept API-breaking changes after 0.39 which meets following criteria: - (only) critical bugfix - not for additional feature or enhancemnet - not cosmetic API cleanup JFYI, there is only one ticket left for the 0.39 milestone which is waiting for validation if we have proper error-handling for all public Interfaces. Best Regards, Daniel -- Daniel Gollub Geschaeftsfuehrer: Ralph Dehner FOSS Developer Unternehmenssitz: Vohburg B1 Systems GmbH Amtsgericht: Ingolstadt Mobil: +49-(0)-160 47 73 970 Handelsregister: HRB 3537 EMail: go...@b1... http://www.b1-systems.de Adresse: B1 Systems GmbH, Osterfeldstraße 7, 85088 Vohburg http://pgpkeys.pca.dfn.de/pks/lookup?op=get&search=0xED14B95C2F8CA78D |
|
From: Chris F. <cd...@fo...> - 2009-09-18 01:18:11
|
On Thu, Sep 17, 2009 at 02:57:11AM +0200, Daniel Gollub wrote:
> Hi,
>
> could you all double-check the public interfaces of the OpenSync trunk API, if
> they fit your needs for you application/plugin ... (those which got exported
> by the OSYNC_EXPORT macro)
Hi Daniel,
I figured I'd give the Barry 0.3x/0.4x plugin a test compile, but I got
stuck before I got there trying to compile the vformat plugin.
The OSyncError** changes to the time API don't seem to have trickled through,
and now, if I understand correctly, there's a lot of opportunity for leaks.
For example, this partial patch shows that there is **error cleanup needed
even inside error handling if() blocks...
Is this the planned way to fix this, or am I missing something?
Thanks,
- Chris
diff --git a/src/xmlformat-recurrence.c b/src/xmlformat-recurrence.c
index f8fffd5..87edf3b 100644
--- a/src/xmlformat-recurrence.c
+++ b/src/xmlformat-recurrence.c
@@ -115,6 +115,7 @@ static void convert_vcal_rrule_countuntil(OSyncXMLField *xmlfield, const char *d
int count;
int offset = 0;
char *until = NULL;
+ OSyncError *error = NULL;
/* COUNT: #20 */
if (sscanf(duration_block, "#%d", &count) == 1) {
@@ -132,12 +133,13 @@ static void convert_vcal_rrule_countuntil(OSyncXMLField *xmlfield, const char *d
*/
if (!osync_time_isutc(duration_block)) {
- struct tm *ttm = osync_time_vtime2tm(duration_block);
- offset = osync_time_timezone_diff(ttm);
+ struct tm *ttm = osync_time_vtime2tm(duration_block, &error);
+ offset = osync_time_timezone_diff(ttm, &error);
+
g_free(ttm);
}
- until = osync_time_vtime2utc(duration_block, offset);
+ until = osync_time_vtime2utc(duration_block, offset, &error);
} else {
until = g_strdup(duration_block);
}
@@ -145,6 +147,11 @@ static void convert_vcal_rrule_countuntil(OSyncXMLField *xmlfield, const char *d
FIXME_xmlfield_set_key_value(xmlfield, "Until", until);
g_free(until);
+
+ if( error != NULL ) {
+ osync_trace(TRACE_ERROR, "%s: %s" , __func__, osync_error_print(&error));
+ osync_error_unref(&error);
+ }
}
OSyncXMLField *convert_vcal_rrule_to_xml(OSyncXMLFormat *xmlformat, VFormatAttribute *attr, const char *rulename, OSyncError **error)
@@ -305,6 +312,7 @@ static char *convert_rrule_vcal_until(const char *until_utc)
{
int offset = 0;
char *until = NULL;
+ OSyncError *error = NULL;
/* UNTIL: 20070515T120000 */
@@ -313,10 +321,15 @@ static char *convert_rrule_vcal_until(const char *until_utc)
* in the same Timezone as the host.
*/
- struct tm *ttm = osync_time_vtime2tm(until_utc);
- offset = osync_time_timezone_diff(ttm);
+ struct tm *ttm = osync_time_vtime2tm(until_utc, &error);
+ offset = osync_time_timezone_diff(ttm, &error);
g_free(ttm);
- until = osync_time_vtime2localtime(until_utc, offset);
+ until = osync_time_vtime2localtime(until_utc, offset, &error);
+
+ if( error != NULL ) {
+ osync_trace(TRACE_ERROR, "%s: %s" , __func__, osync_error_print(&error));
+ osync_error_unref(&error);
+ }
return until;
diff --git a/src/xmlformat-vcalendar.c b/src/xmlformat-vcalendar.c
index 46320b7..76ab132 100644
--- a/src/xmlformat-vcalendar.c
+++ b/src/xmlformat-vcalendar.c
@@ -268,6 +268,7 @@ VFormatAttribute *handle_xml_vcal_alarm_attribute(VFormat *vevent, OSyncXMLField
const char *action = osync_xmlfield_get_key_value(xmlfield, "AlarmAction");
const char *trigger = osync_xmlfield_get_key_value(xmlfield, "AlarmTrigger");
+ OSyncError *error = NULL;
// trigger and action are required
if (trigger && action) {
@@ -318,15 +319,15 @@ VFormatAttribute *handle_xml_vcal_alarm_attribute(VFormat *vevent, OSyncXMLField
}
duration = osync_time_alarmdu2sec(trigger);
- dtstarted = osync_time_vtime2unix(dtstart, 0);
+ dtstarted = osync_time_vtime2unix(dtstart, 0, &error);
dtstarted += duration;
- runtime = osync_time_unix2vtime(&dtstarted);
+ runtime = osync_time_unix2vtime(&dtstarted, &error);
if (!osync_time_isutc(dtstart)) {
osync_trace(TRACE_INTERNAL, "WARNNING: timestamp is not UTC - converting reminder to localtime");
char *utc_runtime = runtime;
- runtime = osync_time_vtime2localtime(utc_runtime, 0);
+ runtime = osync_time_vtime2localtime(utc_runtime, 0, &error);
g_free(utc_runtime);
}
@@ -347,6 +348,14 @@ VFormatAttribute *handle_xml_vcal_alarm_attribute(VFormat *vevent, OSyncXMLField
vformat_add_attribute(vevent, attr);
+ /* Check for any time-related error before returning */
+ if( error != NULL ) {
+ osync_trace(TRACE_INTERNAL, "Error (time-related function failed): %s",
+ osync_error_print(&error));
+ osync_error_unref(&error);
+ return NULL;
+ }
+
return attr;
}
/* END: xml -> vcalendar10 only attributes */
|
|
From: Chris F. <cd...@fo...> - 2009-09-18 03:42:13
|
On Thu, Sep 17, 2009 at 09:17:03PM -0400, Chris Frey wrote:
> OSyncXMLField *convert_vcal_rrule_to_xml(OSyncXMLFormat *xmlformat, VFormatAttribute *attr, const char *rulename, OSyncError **error)
> @@ -305,6 +312,7 @@ static char *convert_rrule_vcal_until(const char *until_utc)
> {
> int offset = 0;
> char *until = NULL;
> + OSyncError *error = NULL;
>
>
[...]
> + until = osync_time_vtime2localtime(until_utc, offset, &error);
> +
> + if( error != NULL ) {
> + osync_trace(TRACE_ERROR, "%s: %s" , __func__, osync_error_print(&error));
> + osync_error_unref(&error);
> + }
Just to refresh my memory, I know that in the usage above, I'm responsible
for unref'ing the error, since I'm the only one that sees it.
But if an OSyncError **error is passed in to my function, am I still
responsible for unref'ing it? This almost seems like the responsibility
of the calling code, but in the sample plugin, the error is unref'd
even inside get_sync_info(). I don't know why the pointer is being passed
in if I'm responsible for unref'ing it.
Thanks,
- Chris
|
|
From: Daniel G. <go...@b1...> - 2009-09-18 07:40:41
|
On Friday 18 September 2009 05:15:37 am Chris Frey wrote: > Just to refresh my memory, I know that in the usage above, I'm responsible > for unref'ing the error, since I'm the only one that sees it. Right. But acutally we should avoid that any error gets lost inside a plugin. So the idea is to pass all error via the OSyncError** parameter to the OpenSync Framework or via osync_context_report_error() if there is a context pointer. > > But if an OSyncError **error is passed in to my function, am I still > responsible for unref'ing it? Thats indeed confusing. If the plugin function only pass OSyncContext* you need to repot the error with osync_context_report_error() and then unref it again ;/ If the function get called with OSyncError** you don't unref it. Does this make sense? Looks pretty confusing ... but i hesistate to touch this interfaces again .... > This almost seems like the responsibility > of the calling code, but in the sample plugin, the error is unref'd The example plugin might be outdated - so if there is anything fishy in it - then it's very likely wrong. > even inside get_sync_info(). I don't know why the pointer is being passed > in if I'm responsible for unref'ing it. > Arrgg indeed. Thats wrong ... thanks for the hint. Best Regards, Daniel -- Daniel Gollub Geschaeftsfuehrer: Ralph Dehner FOSS Developer Unternehmenssitz: Vohburg B1 Systems GmbH Amtsgericht: Ingolstadt Mobil: +49-(0)-160 47 73 970 Handelsregister: HRB 3537 EMail: go...@b1... http://www.b1-systems.de Adresse: B1 Systems GmbH, Osterfeldstraße 7, 85088 Vohburg http://pgpkeys.pca.dfn.de/pks/lookup?op=get&search=0xED14B95C2F8CA78D |
|
From: Chris F. <cd...@fo...> - 2009-09-18 22:54:43
|
On Fri, Sep 18, 2009 at 09:42:18AM +0200, Daniel Gollub wrote: > If the plugin function only pass OSyncContext* you need to repot the error > with osync_context_report_error() and then unref it again ;/ > > If the function get called with OSyncError** you don't unref it. > > Does this make sense? Looks pretty confusing ... but i hesistate to touch this > interfaces again .... That clears things up for me, thanks! - Chris |
|
From: Rainer D. <rd...@we...> - 2009-09-20 14:06:50
|
Hi Daniel, Am Donnerstag, 17. September 2009 schrieb Daniel Gollub: > Hi, > > could you all double-check the public interfaces of the OpenSync trunk API, > if they fit your needs for you application/plugin ... (those which got > exported by the OSYNC_EXPORT macro) > > I plan to _soft_ freeze thursday evening/night (CEST) the OpenSync API and > do a quick accepteance test with various "hot-requested" plugins ... and > port those which aren't yet ported. If you need assistance porting - please > drop a mail on opensync-devel mailing list. > > By "soft freeze" i plan to accept API-breaking changes after 0.39 which > meets following criteria: > > - (only) critical bugfix > > > - not for additional feature or enhancemnet > - not cosmetic API cleanup > > > JFYI, there is only one ticket left for the 0.39 milestone which is waiting > for validation if we have proper error-handling for all public Interfaces. I have devices which use irmc-sync and syncml-obex-client. Do you think it is already worth to do testing with these plugins? Thanks, Rainer -- Rainer Dorsch Lärchenstr. 6 D-72135 Dettenhausen 07157-734133 email: rd...@we... jabber: rd...@ja... GPG Fingerprint: 5966 C54C 2B3C 42CC 1F4F 8F59 E3A8 C538 7519 141E Full GPG key: http://pgp.mit.edu/ |
Thanks for helping keep SourceForge clean.
X