Re: [Opensync-devel] osync_objtype_sink_get_userdata intentionally not exported?

[Chris F. <cd...@fo...>]

On Fri, May 27, 2011 at 01:24:28AM -0300, Luiz Angelo Daros de Luca wrote:
> So, I have some suggestions:
> 
> * make all _new function behave the same: only malloc or malloc and
> also define mandatory attributes.
> * if _new only mallocs, create the necessary setters and getters for
> each of the attributes

I'm not entirely sure that objformat's name is supposed to change.
These names show up in the osynctool --listformats output.

I favour APIs that make it impossible to do the wrong thing, and
I suspect that this _new(), with attributes, is doing that.


> * add a set/get_data to objformat.

I can see adding a get_userdata(), but the set_userdata() is handled
by the initialize callback.  By adding set_userdata(), we'd introduce
a potential race condition during setup, depending which was called
first.


> * Some callbacks returns values while others not. Why return
> osync_bool if error is enough to tell if the function
> worked? Why force initialization to return a void* if, maybe, the
> initialization is not used to create a struct for data?

If you could point out concrete examples, we could make sure.
I'm all for simplifying the API, but maybe the library checks the
return values on some of these.  We'd have to reverse engineer the
reasoning from the code. :-)


> * Choose one unique data field name. I found three: user_data, userdata and data

I'd go with userdata, since that's self-documenting.  Patches welcome.


> * Plugin callback define methods in plugin
> (set_initialize/finalize/discovery) does not have a suffix like in
> sink and others. Maybe a osync_plugin_set_initialize_func would sound
> better.

Sounds good to me.  Patches welcome.


>  * Is there any difference in osync_format and osync_objformat? I know
> that there is only osync_format_env but this format isn't really
> objformat?

I think the goal was to somehow link the naming of objformat with
objtype, instead of just the plain "format" and "type".  There are
a few top level objtypes, but there can be many objformats for each
objtype.

osync_format_ functions appear to be the namespace used for conversions.

As I understanding, there are objtypes, each objtype can have multiple
objformats, and then to make use of them, you can create format_env
environments with which to convert between objformats.

I think the naming is ok here... maybe not consistent from a function
naming perspective, but from a meaning perspective, it helps to
remember the hierarchy of types.

Let me know if you disagree. :-)

Thanks!
- Chris