[API] Check public interfaces for proper error reporting

[dgollub]

Complex public interfaces which call internal functions which return OSyncError* on an error should also have OSyncError* in the parameter list.

So frontends/applications don't hide any errors.

Tracing should be only used in very extrem debugging...

[dgollub]

Example: osync_marshal_read_string This function calls osync_try_malloc0() which acutally could fail and set a OSyncError strut. Instead it's failing completely silently.

Or osync_rand_str() as well allocate which might fail and can't return a error why on a OOM.

There are very likely also many public API functions which return bool false or pointer NULL and which should fill soem "error" struct. If there are severail reason why a function false or return NULL.

[dgollub]

Maybe this helps to review only the likely interfaces:

grep -r OSYNC_EXPORT * | grep -v OSyncError |grep -v _unref | grep -v _ref | grep -v _get | grep -v _set_ | grep -v "#define" | grep -v "opensync_list\.h" | grep -v _has_ | grep -v _is_ | grep -v "OSyncContext \*"  | grep -v osync_trace | grep -v "_enable(" 

Repots approx. 127 interfaces to check. I guess some you can review/ignore just by the same - so I did with soem functions already when assembling this command line.

[dgollub]

We disccused on IRC to introduce "osync_strdup_error":

osync_strdup() is used by:

Referenced by osync_archive_load_changes(), osync_change_clone(), osync_change_set_hash(), osync_change_set_uid(), osync_converter_path_set_config(), osync_custom_filter_new(), osync_data_clone(), osync_data_set_objtype(), osync_db_query_single_string(), osync_db_query_table(), osync_entry_engine_convert(), osync_filter_new(), osync_filter_new_custom(), osync_filter_set_config(), osync_group_env_load_groups(), osync_group_load(), osync_group_set_configdir(), osync_group_set_name(), osync_hashtable_update_change(), osync_mapping_entry_set_uid(), osync_member_add_alternative_objtype(), osync_member_get_all_objformats(), osync_member_load(), osync_member_set_configdir(), osync_member_set_name(), osync_member_set_pluginname(), osync_module_load(), osync_objformat_new(), osync_objformat_sink_new(), osync_objformat_sink_set_config(), osync_objtype_sink_new(), osync_objtype_sink_set_name(), osync_objtype_sink_set_preferred_format(), osync_plugin_advancedoption_add_valenum(), osync_plugin_advancedoption_param_add_valenum(), osync_plugin_advancedoption_param_set_displayname(), osync_plugin_advancedoption_param_set_name(), osync_plugin_advancedoption_param_set_value(), osync_plugin_advancedoption_set_displayname(), osync_plugin_advancedoption_set_name(), osync_plugin_advancedoption_set_value(), osync_plugin_authentication_set_password(), osync_plugin_authentication_set_reference(), osync_plugin_authentication_set_username(), osync_plugin_connection_bt_set_addr(), osync_plugin_connection_bt_set_sdpuuid(), osync_plugin_connection_irda_set_service(), osync_plugin_connection_net_set_address(), osync_plugin_connection_net_set_dnssd(), osync_plugin_connection_net_set_protocol(), osync_plugin_connection_serial_set_devicenode(), osync_plugin_connection_usb_set_productid(), osync_plugin_connection_usb_set_vendorid(), osync_plugin_info_set_configdir(), osync_plugin_info_set_groupname(), osync_plugin_set_description(), osync_plugin_set_longname(), osync_plugin_set_name(), osync_queue_new(), osync_sink_state_get(), osync_updater_new(), osync_updater_set_updates_directory(), osync_version_new(), osync_version_set_firmwareversion(), osync_version_set_hardwareversion(), osync_version_set_identifier(), osync_version_set_modelversion(), osync_version_set_plugin(), osync_version_set_priority(), osync_version_set_softwareversion(), and osync_version_set_vendor().

[bricks]

capabilities/opensync_capabilities.h:OSYNC_EXPORT osync_bool osync_capabilities_assemble(OSyncCapabilities *capabilities, char **buffer, int *size);
capabilities/opensync_capability.h:OSYNC_EXPORT void osync_capability_add_key(OSyncCapability *capability, const char *name);

Both functions are using libxml2 internally. The documentation doesn't include anything about error conditions. Don't know if there is any additional error handling needed.

[bricks]

client/opensync_client.h:OSYNC_EXPORT void osync_client_run_and_block(OSyncClient *client);

This function only calls g_main_loop_run. I guess there is no error handling needed.

[bricks]

common/opensync_string.h:OSYNC_EXPORT char *osync_strreplace(const char *input, const char *delimiter, const char *replacement);
common/opensync_string.h:OSYNC_EXPORT char *osync_strdup(const char *str);
common/opensync_string.h:OSYNC_EXPORT char *osync_strdup_printf(const char *format, ...);

These functions internally use glib functions that call abort if no memory is available -> no error handling

[bricks]

common/opensync_string.h:OSYNC_EXPORT char *osync_rand_str(int maxlength);

Calls retchar = sync_try_malloc0 and afterwards osync_return_val_if_fail(retchar, NULL); Therefore it could be added an OSyncError.

[bricks]

common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_int(OSyncMarshal *marshal, int value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_uint(OSyncMarshal *marshal, unsigned int value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_long_long_int(OSyncMarshal *marshal, long long int value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_string(OSyncMarshal *marshal, const char *value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_data(OSyncMarshal *marshal, const void *value, unsigned int size);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_write_buffer(OSyncMarshal *marshal, const void *value, unsigned int size);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_int(OSyncMarshal *marshal, int *value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_uint(OSyncMarshal *marshal, unsigned int *value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_long_long_int(OSyncMarshal *marshal, long long int *value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_string(OSyncMarshal *marshal, char **value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_data(OSyncMarshal *marshal, void *value, unsigned int size);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_const_data(OSyncMarshal *marshal, void **value, unsigned int size);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_const_string(OSyncMarshal *marshal, const char **value);
common/opensync_marshal.h:OSYNC_EXPORT void osync_marshal_read_buffer(OSyncMarshal *marshal, void **value, unsigned int *size);

These functions are using different patterns. osync_marshal_read_buffer uses g_malloc0 that internally will abort if there isn't enough memory. osync_marshal_read_data assumes that there is already memory allocated and osync_marshal_read_string uses osync_try_malloc0 which won't abort in case of not enough memory. All other functions don't need any further error handling. So what to do here?

[bricks]

common/opensync_memory.h:OSYNC_EXPORT void osync_free(void *ptr);
data/opensync_change.h:OSYNC_EXPORT OSyncConvCmpResult osync_change_compare(OSyncChange *leftchange, OSyncChange *rightchange);

don't not need any error handling

[bricks]

engine/opensync_mapping_engine.h:OSYNC_EXPORT osync_bool osync_mapping_engine_supports_use_latest(OSyncMappingEngine *engine);

This function calls internal "_osync_mapping_engine_get_latest_entry(engine, NULL);" The comment before this call is /* we ignore the error for now ... it's just a test if it would be possible/supported. */ Don't know if the function signature should be osync_mapping_engine_supports_use_latest(OSyncMappingEngine *engine, OSyncError error);

[bricks]

engine/opensync_engine.h:OSYNC_EXPORT OSyncObjEngine *osync_engine_find_objengine(OSyncEngine *engine, const char *objtype);
engine/opensync_mapping_engine.h:OSYNC_EXPORT OSyncChange *osync_mapping_engine_member_change(OSyncMappingEngine *engine, long long int memberid);
engine/opensync_mapping_engine.h:OSYNC_EXPORT OSyncMember *osync_mapping_engine_change_find_member(OSyncMappingEngine *engine, OSyncChange *change);
engine/opensync_mapping_engine.h:OSYNC_EXPORT osync_bool osync_mapping_engine_supports_ignore(OSyncMappingEngine *engine);

No error handling is needed in these functions

[bricks]

engine/opensync_obj_engine.h:OSYNC_EXPORT void osync_obj_engine_finalize(OSyncObjEngine *engine);
format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_objformat(OSyncFormatEnv *env, OSyncObjFormat *format);
format/opensync_format_env.h:OSYNC_EXPORT OSyncObjFormat *osync_format_env_find_objformat(OSyncFormatEnv *env, const char *name);
format/opensync_format_env.h:OSYNC_EXPORT OSyncList *osync_format_env_find_converters(OSyncFormatEnv *env, OSyncObjFormat *sourceformat, OSyncObjFormat *targetformat);
format/opensync_format_env.h:OSYNC_EXPORT OSyncList *osync_format_env_find_converters(OSyncFormatEnv *env, OSyncObjFormat *sourceformat, OSyncObjFormat *targetformat);
format/opensync_format_env.h:OSYNC_EXPORT OSyncObjFormat *osync_format_env_detect_objformat(OSyncFormatEnv *env, OSyncData *data);

No error handling is needed in these functions

[bricks]

format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_converter(OSyncFormatEnv *env, OSyncFormatConverter *converter);

This function calls OSyncFormatConverter *conv = osync_converter_new_detector(osync_converter_get_targetformat(converter), osync_converter_get_sourceformat(converter), NULL, NULL); for detectors. Therefore a OSyncError parameter should be added'''

[bricks]

format/opensync_converter.h:OSYNC_EXPORT OSyncObjFormat *osync_converter_detect(OSyncFormatConverter *converter, OSyncData *data);

This function includes the commands:

	if (detector->type != OSYNC_CONVERTER_DETECTOR) {
		osync_trace(TRACE_EXIT, "%s: Not a detector", __func__);
		return NULL;
	}

I am not sure if this is an error case.

[bricks]

format/opensync_converter.h:OSYNC_EXPORT osync_bool osync_converter_matches(OSyncFormatConverter *converter, OSyncData *data);
format/opensync_converter.h:OSYNC_EXPORT void osync_converter_path_add_edge(OSyncFormatConverterPath *path, OSyncFormatConverter *edge);
format/opensync_converter.h:OSYNC_EXPORT void osync_converter_finalize(OSyncFormatConverter *converter);
format/opensync_objformat.h:OSYNC_EXPORT char *osync_objformat_print(OSyncObjFormat *format, const char *data, unsigned int size);

No error handling is needed in these functions

[bricks]

format/opensync_time.h:OSYNC_EXPORT char *osync_time_timestamp(const char *vtime);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_datestamp(const char *vtime); 
format/opensync_time.h:OSYNC_EXPORT osync_bool osync_time_isdate(const char *vformat);
format/opensync_time.h:OSYNC_EXPORT osync_bool osync_time_isutc(const char *vformat);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_vtime2tm(const char *vtime);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_tm2vtime(const struct tm *time, osync_bool is_utc);
format/opensync_time.h:OSYNC_EXPORT time_t osync_time_vtime2unix(const char *vtime, int offset);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_unix2vtime(const time_t *timestamp);
format/opensync_time.h:OSYNC_EXPORT time_t osync_time_localtm2unix(const struct tm *localtime);
format/opensync_time.h:OSYNC_EXPORT time_t osync_time_utctm2unix(const struct tm *utctime);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_unix2localtm(const time_t *timestamp);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_unix2utctm(const time_t *timestamp);
format/opensync_time.h:OSYNC_EXPORT int osync_time_timezone_diff(const struct tm *local);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_tm2utc(const struct tm *ltime, int offset);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_tm2localtime(const struct tm *utime, int offset);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_vtime2utc(const char* localtime, int offset);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_vtime2localtime(const char* utc, int offset);
format/opensync_time.h:OSYNC_EXPORT int osync_time_utcoffset2sec(const char *offset);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_vcal2localtime(const char *vcal);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_vcal2utc(const char *vcal);
format/opensync_time.h:OSYNC_EXPORT char *osync_time_sec2alarmdu(int seconds);
format/opensync_time.h:OSYNC_EXPORT int osync_time_alarmdu2sec(const char *alarm);
format/opensync_time.h:OSYNC_EXPORT int osync_time_str2wday(const char *swday);
format/opensync_time.h:OSYNC_EXPORT struct tm *osync_time_relative2tm(const char *byday, const int bymonth, const int year);

The time api makes heavy use of g_malloc0, g_string_new and g_strdup functions. These functions are going to call abort() if no more memory is available. Therefore no OSyncError parameter is necessary.

[bricks]

group/opensync_group.h:OSYNC_EXPORT OSyncLockState osync_group_lock(OSyncGroup *group);

This function includes two calls of osync_trace with TRACE_EXIT_ERROR also the call osync_trace(TRACE_EXIT, "%s: Unable to open: %s", func, g_strerror(errno)); should be an error. Therefore this function should have and OSyncError parameter

[bricks]

group/opensync_group.h:OSYNC_EXPORT void osync_group_unlock(OSyncGroup *group);
group/opensync_group.h:OSYNC_EXPORT void osync_group_add_member(OSyncGroup *group, OSyncMember *member);
group/opensync_group.h:OSYNC_EXPORT void osync_group_remove_member(OSyncGroup *group, OSyncMember *member);
group/opensync_group.h:OSYNC_EXPORT OSyncMember *osync_group_find_member(OSyncGroup *group, long long int id);
group/opensync_group.h:OSYNC_EXPORT osync_bool osync_group_objtype_enabled(OSyncGroup *group, const char *objtype);

No error handling is necessary in these functions

[bricks]

group/opensync_member.h:OSYNC_EXPORT void osync_member_add_objtype_sink(OSyncMember *member, OSyncObjTypeSink *sink);
group/opensync_member.h:OSYNC_EXPORT OSyncObjTypeSink *osync_member_find_objtype_sink(OSyncMember *member, const char *objtype);
group/opensync_member.h:OSYNC_EXPORT osync_bool osync_member_objtype_enabled(OSyncMember *member, const char *objtype);
group/opensync_member.h:OSYNC_EXPORT void osync_member_flush_objtypes(OSyncMember *member);

No error handling is necessary in these functions

[bricks]

group/opensync_member.h:OSYNC_EXPORT void osync_member_add_objformat(OSyncMember *member, const char *objtype, const char *format);
group/opensync_member.h:OSYNC_EXPORT void osync_member_add_objformat_with_config(OSyncMember *member, const char *objtype, const char *format, const char *format_config);

Both functions call osync_objformat_sink_new(format, NULL); and as the comment in osync_member_add_objformat says /* TODO: handle error */

[bricks]

group/opensync_group_env.h:OSYNC_EXPORT OSyncGroup *osync_group_env_find_group(OSyncGroupEnv *env, const char *name);
group/opensync_group_env.h:OSYNC_EXPORT void osync_group_env_remove_group(OSyncGroupEnv *env, OSyncGroup *group);
helper/opensync_hashtable.h:OSYNC_EXPORT void osync_hashtable_foreach(OSyncHashTable *table, OSyncHashtableForEach func, void *user_data);
helper/opensync_hashtable.h:OSYNC_EXPORT void osync_hashtable_update_change(OSyncHashTable *table, OSyncChange *change);

No error handling is necessary in these functions

[bricks]

mapping/opensync_mapping.h:OSYNC_EXPORT void osync_mapping_add_entry(OSyncMapping *mapping, OSyncMappingEntry *entry);
mapping/opensync_mapping.h:OSYNC_EXPORT void osync_mapping_remove_entry(OSyncMapping *mapping, OSyncMappingEntry *entry);
mapping/opensync_mapping.h:OSYNC_EXPORT OSyncMappingEntry *osync_mapping_find_entry_by_member_id(OSyncMapping *mapping, long long int memberid);
mapping/opensync_mapping_table.h:OSYNC_EXPORT void osync_mapping_table_close(OSyncMappingTable *table);
mapping/opensync_mapping_table.h:OSYNC_EXPORT OSyncMapping *osync_mapping_table_find_mapping(OSyncMappingTable *table, long long int id);
mapping/opensync_mapping_table.h:OSYNC_EXPORT void osync_mapping_table_add_mapping(OSyncMappingTable *table, OSyncMapping *mapping);
mapping/opensync_mapping_table.h:OSYNC_EXPORT void osync_mapping_table_remove_mapping(OSyncMappingTable *table, OSyncMapping *mapping);
mapping/opensync_mapping_entry.h:OSYNC_EXPORT osync_bool osync_mapping_entry_matches(OSyncMappingEntry *entry, OSyncChange *change);
mapping/opensync_mapping_entry.h:OSYNC_EXPORT void osync_mapping_entry_update(OSyncMappingEntry *entry, OSyncChange *change);

No error handling is necessary in these functions

[bricks]

plugin/opensync_objtype_sink.h:OSYNC_EXPORT void osync_objtype_sink_enable_state_db(OSyncObjTypeSink *sink, osync_bool enable);
plugin/opensync_objtype_sink.h:OSYNC_EXPORT void osync_objtype_sink_enable_hashtable(OSyncObjTypeSink *sink, osync_bool enable);
plugin/opensync_objtype_sink.h:OSYNC_EXPORT OSyncObjFormatSink *osync_objtype_sink_find_objformat_sink(OSyncObjTypeSink *sink, OSyncObjFormat *objformat);
plugin/opensync_objtype_sink.h:OSYNC_EXPORT void osync_objtype_sink_add_objformat_sink(OSyncObjTypeSink *sink, OSyncObjFormatSink *objformatsink);
plugin/opensync_objtype_sink.h:OSYNC_EXPORT void osync_objtype_sink_remove_objformat_sink(OSyncObjTypeSink *sink, OSyncObjFormatSink *objformatsink);
plugin/opensync_plugin_config.h:OSYNC_EXPORT void osync_plugin_config_add_advancedoption(OSyncPluginConfig *config, OSyncPluginAdvancedOption *option);
plugin/opensync_plugin_config.h:OSYNC_EXPORT void osync_plugin_config_remove_advancedoption(OSyncPluginConfig *config, OSyncPluginAdvancedOption *option);
plugin/opensync_plugin_config.h:OSYNC_EXPORT OSyncPluginResource *osync_plugin_config_find_active_resource(OSyncPluginConfig *config, const char *objtype);
plugin/opensync_plugin_config.h:OSYNC_EXPORT void osync_plugin_config_add_resource(OSyncPluginConfig *config, OSyncPluginResource *resource);
plugin/opensync_plugin_config.h:OSYNC_EXPORT void osync_plugin_config_remove_resource(OSyncPluginConfig *config, OSyncPluginResource *resource);
plugin/opensync_plugin_config.h:OSYNC_EXPORT void osync_plugin_config_flush_resources(OSyncPluginConfig *config);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT OSyncPluginAdvancedOptionType osync_plugin_advancedoption_type_string_to_val(const char *typestr);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_add_parameter(OSyncPluginAdvancedOption *option, OSyncPluginAdvancedOptionParameter *param);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_remove_parameter(OSyncPluginAdvancedOption *option, OSyncPluginAdvancedOptionParameter *param);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_add_valenum(OSyncPluginAdvancedOption *option, const char *value);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_remove_valenum(OSyncPluginAdvancedOption *option, const char *value);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_param_add_valenum(OSyncPluginAdvancedOptionParameter *param, const char *value);
plugin/opensync_plugin_advancedoptions.h:OSYNC_EXPORT void osync_plugin_advancedoption_param_remove_valenum(OSyncPluginAdvancedOptionParameter *param, const char *value);
plugin/opensync_plugin_env.h:OSYNC_EXPORT void osync_plugin_env_register_plugin(OSyncPluginEnv *env, OSyncPlugin *plugin);
plugin/opensync_plugin_env.h:OSYNC_EXPORT OSyncPlugin *osync_plugin_env_find_plugin(OSyncPluginEnv *env, const char *name);
plugin/opensync_plugin.h:OSYNC_EXPORT void osync_plugin_finalize(OSyncPlugin *plugin, void *data);
plugin/opensync_plugin_info.h:OSYNC_EXPORT OSyncObjTypeSink *osync_plugin_info_find_objtype(OSyncPluginInfo *info, const char *name);
plugin/opensync_plugin_info.h:OSYNC_EXPORT void osync_plugin_info_add_objtype(OSyncPluginInfo *info, OSyncObjTypeSink *sink);
plugin/opensync_plugin_resource.h:OSYNC_EXPORT void osync_plugin_resource_add_objformat_sink(OSyncPluginResource *resource, OSyncObjFormatSink *formatsink);
plugin/opensync_plugin_resource.h:OSYNC_EXPORT void osync_plugin_resource_remove_objformat_sink(OSyncPluginResource *resource, OSyncObjFormatSink *formatsink);

No error handling is necessary for the plugin api

[bricks]

xmlformat/opensync_xmlfield.h:OSYNC_EXPORT void osync_xmlfield_delete(OSyncXMLField *xmlfield);
xmlformat/opensync_xmlfield.h:OSYNC_EXPORT void osync_xmlfield_unlink(OSyncXMLField *xmlfield);
xmlformat/opensync_xmlfield.h:OSYNC_EXPORT void osync_xmlfield_adopt_xmlfield_before_field(OSyncXMLField *xmlfield, OSyncXMLField *to_link);
xmlformat/opensync_xmlfield.h:OSYNC_EXPORT void osync_xmlfield_sort(OSyncXMLField *xmlfield);
xmlformat/opensync_xmlfieldlist.h:OSYNC_EXPORT void osync_xmlfieldlist_free(OSyncXMLFieldList *xmlfieldlist);
xmlformat/opensync_xmlfieldlist.h:OSYNC_EXPORT OSyncXMLField *osync_xmlfieldlist_item(OSyncXMLFieldList *xmlfieldlist, unsigned int index);
xmlformat/opensync_xmlformat.h:OSYNC_EXPORT osync_bool osync_xmlformat_assemble(OSyncXMLFormat *xmlformat, char **buffer, unsigned int *size);
xmlformat/opensync_xmlformat.h:OSYNC_EXPORT void osync_xmlformat_sort(OSyncXMLFormat *xmlformat);
xmlformat/opensync_xmlformat.h:OSYNC_EXPORT unsigned int osync_xmlformat_size();

These functions are using libxml2 internally. But it seems that error handling shouldn't be necessary

[bricks]

xmlformat/opensync_xmlfield.h:OSYNC_EXPORT void osync_xmlfield_add_key_value(OSyncXMLField *xmlfield, const char *key, const char *value);

This function calls osync_xmlfield_new_xmlfield(xmlfield, cur, NULL); Therefore an error handling is necessary here

[bricks]

(In [5671]) Added an OSyncError parameter for osync_format_env_register_converter refs #1087

[bricks]

(In [5673]) added an OSyncError parameter to osync_group_lock refs #1087

[bricks]

(In [5674]) added OSyncError parameter to osync_member_add_objformat and osync_member_add_objformat_with_config refs #1087

[dgollub]

Björn, could you do a very last run through any summaries which Interface still needs to be changed - or require any decision or attention from me?

I want to close this ticket this thursday evening latest.

[dgollub]

(In [5769]) Add OSyncError to parameter list for

• osync_xmlfield_add_key_value
• osync_xmlfield_set_key_value
• osync_xmlfield_sort

refs #1087

[dgollub]

Replying to bricks:

engine/opensync_mapping_engine.h:OSYNC_EXPORT osync_bool osync_mapping_engine_supports_use_latest(OSyncMappingEngine *engine);

This function calls internal "_osync_mapping_engine_get_latest_entry(engine, NULL);" The comment before this call is /* we ignore the error for now ... it's just a test if it would be possible/supported. */ Don't know if the function signature should be osync_mapping_engine_supports_use_latest(OSyncMappingEngine *engine, OSyncError error);

Nope - we ignore here the error on purpose.

[dgollub]

Still TODO:

• osync_format_env_register_converter comment:16
• osync_group_lock comment:20
• osync_member_add_objformat comment:23
• osync_member_add_objformat_with_config comment:23

[dgollub]

Replying to dgollub:

Still TODO:

• osync_format_env_register_converter comment:16

We should introduce here a pattern for all osync_*_register_*() functions - like this:

• osync_bool osync_XXXX_register_YYYYY(OSyncXxxxx *, OSyncYyyyy *, OSyncError )
• even for those function which might not need an error parameter yet
  • but this makes the API more future proof
• affected interfaces:

  dgollub@marvin:~/projects/OpenSync/opensync> grep -r register_ opensync/* | grep OSYNC_EXPORT | grep -v svn
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_objformat(OSyncFormatEnv *env, OSyncObjFormat *format);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT osync_bool osync_format_env_register_converter(OSyncFormatEnv *env, OSyncFormatConverter *converter, OSyncError **error);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_caps_converter(OSyncFormatEnv *env, OSyncCapsConverter *converter, OSyncError **error);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_merger(OSyncFormatEnv *env, OSyncMerger *merger); 
  opensync/plugin/opensync_plugin_env.h:OSYNC_EXPORT void osync_plugin_env_register_plugin(OSyncPluginEnv *env, OSyncPlugin *plugin);
  

• Requires changes on _all_ plugins!

• osync_group_lock comment:20

is already present -> done

• osync_member_add_objformat comment:23
• osync_member_add_objformat_with_config comment:23

both already have an error struct in their parameter list

[dgollub]

(In [5772]) Make plugin/format enviroment API more future-proof and add for all register interfaces an error struct to the parameter list

refs #1087, comment:36

[dgollub]

(In [5773]) Port xmlformat plugin to format_env register API changes. See #1087

[dgollub]

(In [5774]) Port vformat plugin to format_env register API changes. See #1087

[dgollub]

(In [5775]) Port file-sync and file format plugin to format_env and plugin_env register API changes. See #1087

[dgollub]

(In [5776]) Port evo2-sync and evo2 format plugin to format_env and plugin_env register API changes. See #1087

[dgollub]

(In [5777]) Port syncml plugin to plugin_env register API changes. See #1087

[dgollub]

(In [5778]) Port example plugins to format_env and plugin_env register API changes. See #1087

[dgollub]

Replying to dgollub:

Replying to dgollub:

Still TODO:

• osync_format_env_register_converter comment:16

We should introduce here a pattern for all osync_*_register_*() functions - like this:

• osync_bool osync_XXXX_register_YYYYY(OSyncXxxxx *, OSyncYyyyy *, OSyncError )
• even for those function which might not need an error parameter yet
  • but this makes the API more future proof
• affected interfaces:

  dgollub@marvin:~/projects/OpenSync/opensync> grep -r register_ opensync/* | grep OSYNC_EXPORT | grep -v svn
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_objformat(OSyncFormatEnv *env, OSyncObjFormat *format);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT osync_bool osync_format_env_register_converter(OSyncFormatEnv *env, OSyncFormatConverter *converter, OSyncError **error);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_caps_converter(OSyncFormatEnv *env, OSyncCapsConverter *converter, OSyncError **error);
  opensync/format/opensync_format_env.h:OSYNC_EXPORT void osync_format_env_register_merger(OSyncFormatEnv *env, OSyncMerger *merger); 
  opensync/plugin/opensync_plugin_env.h:OSYNC_EXPORT void osync_plugin_env_register_plugin(OSyncPluginEnv *env, OSyncPlugin *plugin);
  

[...]

Done.

Björn, please double-check if there is anything missing from your summary which requires any changes. If we have checked all public interface for error reporting we should close this ticket ASAP.

[bricks]

Daniel, thanks for looking at this ticket.
Imho these action points are still missing:
osync_rand_str  http://opensync.org/ticket/1087#comment:10
osync_marshal_xyz  http://opensync.org/ticket/1087#comment:11
osync_converter_detect  http://opensync.org/ticket/1087#comment:17

In osync_converter_detect I am completely unsure if an error handling is necessary. Maybe returning NULL is intended and expected.

[dgollub]

(In [5779]) Add OSyncError to parameter list of osync_rand_str

See #1087, comment:10

[dgollub]

(In [5780]) Make OSyncMarshal API more future proof and apply a common API pattern:

osync_bool osync_masrhal_XXXX(...., OSyncError)

refs #1087, comment:11

[dgollub]

Replying to bricks:

Daniel, thanks for looking at this ticket.
Imho these action points are still missing:
osync_rand_str  http://opensync.org/ticket/1087#comment:10

fixed, r5779

osync_marshal_xyz  http://opensync.org/ticket/1087#comment:11

fixed, r5780

osync_converter_detect  http://opensync.org/ticket/1087#comment:17

In osync_converter_detect I am completely unsure if an error handling is necessary. Maybe returning NULL is intended and expected.

Your last assumption is correct - no action required: this is intended.

[dgollub]

From IRC chat with Björn:

• replace all code which uses g_malloc0 and replace it by osync_try_malloc0
• adapt functions to pass OSyncError
• exception g_malloc0 use in OSyncError* creation

[dgollub]

(In [5783]) Add OSyncError struct to:

• osync_xmlformat_sort
• osync_xmlformat_assemble

To make those interface more future safe

refs #1087

[dgollub]

22:38 < dgollub> so ... is anything left?
22:38 <@bricks> i don't thinks so :)
22:38 <@bricks> api should be fine
22:38 <@bricks> finally