Preparing for OpenSync API 0.40 soft-freeze

[Daniel Gollub-3]

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: [phone]         Handelsregister:   HRB 3537
EMail: gollub@...          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

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Chris Frey-2]

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 */



------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Chris Frey-2]

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


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub-3]

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: [phone]         Handelsregister:   HRB 3537
EMail: gollub@...          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

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Chris Frey-2]

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


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Rainer Dorsch]

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: rdorsch@...
jabber: rdorsch@...
GPG Fingerprint: 5966 C54C 2B3C 42CC 1F4F  8F59 E3A8 C538 7519 141E
Full GPG key: http://pgp.mit.edu/


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel