opensync-devel

[Opensync-devel] [osynctool] osynctool_list_formats and other functions

[Jakub M. <ja...@o2...>]

I started my "devel path" from looking through the osynctool.c
(http://svn.opensync.org/osynctool/trunk/tools/osynctool.c)

And now (at the very beginning) I have some questions:

1. Function [line: 164]:
   static osync_bool osynctool_list_formats(OSyncFormatEnv *env, OSyncError **error)
   
Argument "error" is never used in the function. Will it be needed in the future? Do we consider as a bug?

2. Some function return "void" and use "osync_trace", while other return "osync_bool" (and do not trace).
Are there any specific rules? When do we need trace? 
"osynctool_list_formats" and "osynctool_list_plugins" seem to be very similar, but they behave differently in this matter.


Thanks in advance for answering my questions. I promise there will be some more ;)

Bloosky
ja...@o2...

Re: [Opensync-devel] [osynctool] osynctool_list_formats and other functions

[Henrik /K. <he...@ka...>]

Hello Jakub!

Great to see a new developer on the project! Welcome!

I am by no means an authoritative source on OpenSync, but I hope my 
comments below will be helpful.

> 1. Function [line: 164]:
>    static osync_bool osynctool_list_formats(OSyncFormatEnv *env, OSyncError **error)
>    
> Argument "error" is never used in the function. Will it be needed in the future? Do we consider as a bug?
>   
No, this is not a bug.
Many functions have an OSyncError** parameter.
Some time in the future the implementation may change  to  something 
which would indeed generate an error.
And we could still keep the API intact.
> 2. Some function return "void" and use "osync_trace", while other return "osync_bool" (and do not trace).
> Are there any specific rules? When do we need trace? 
> "osynctool_list_formats" and "osynctool_list_plugins" seem to be very similar, but they behave differently in this matter.
>   
In my opinion there is no relation between trace and return values.

Any function which has anything interesting to say should osync_trace.

Any function which could "fail" should
(1) either return an osync_bool or a pointer which could be checked for 
NULL.
(2) have an OSyncError parameter
(3) set the OSyncError in case it returns false or NULL.

/Henrik

Re: [Opensync-devel] [osynctool] osynctool_list_formats and other functions

[Daniel G. <go...@b1...>]

On Wednesday 28 October 2009 10:21:42 pm Henrik /KaarPoSoft wrote:
> In my opinion there is no relation between trace and return values.

Right, there isn't any.

> 
> Any function which has anything interesting to say should osync_trace.

But this only counts for OpenSync API code ...  you can use what ever you want 
in your OpenSync based application (e.g. osynctool, ...)

You don't have to use osync_trace() in your plugin ... but most OpenSync 
developer or used to osync-trace files already ... (even if no one knows why 
they have those strange file names and why there are soooo many of them ;))

> 
> Any function which could "fail" should
> (1) either return an osync_bool or a pointer which could be checked for 
> NULL.
> (2) have an OSyncError parameter
> (3) set the OSyncError in case it returns false or NULL.
> 

This counts only for OpenSync API ... we don't care about coding style outside 
of the OpenSync API. But i recommend that all OpenSync based application 
should take care about function which could fail and provide a OSyncError**

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

Re: [Opensync-devel] [osynctool] osynctool_list_formats and other functions

[Daniel G. <go...@b1...>]

On Wednesday 28 October 2009 08:51:03 pm Jakub Marczyński wrote:
> I started my "devel path" from looking through the osynctool.c
> (http://svn.opensync.org/osynctool/trunk/tools/osynctool.c)

Cool, thanks for joining us! ;)

> 
> And now (at the very beginning) I have some questions:
> 
> 1. Function [line: 164]:
>    static osync_bool osynctool_list_formats(OSyncFormatEnv *env, OSyncError
>  **error)
> 
> Argument "error" is never used in the function. Will it be needed in the
>  future? Do we consider as a bug?

First of all ... osynctool_ is not part of the OpenSync API. It's only 
osynctool code ...

Also keep in mind osynctool is "only" an implementation reference and a tool 
for tests/developers to test the OpenSync API ...

But back to the actual topic ... osynctool_list_formts never calls a function 
which may raise an expection or so which fills a OSyncError struct ...

So actually osynctool could be changed to:

void osynctool_list_formats(OSyncFormatenv *env)


It's not a bug - just obsolete, ugly code in osynctool - not OpenSync ;)



> 
> 2. Some function return "void" and use "osync_trace", while other return
>  "osync_bool" (and do not trace). Are there any specific rules? When do we
>  need trace?

Since osynctool stands for all other application based on the OpenSync API, 
you can use what ever debugging/tracing API you want to use. E.g. use QDebug 
or something else ... outside of OpenSync API code you don't have to use 
osync_trace ...

Same for plugins, you don't have to use osync_trace() .. but you can if you 
like.

The rules for code inside the OpenSync API: there is none.
We usually place osync_trace() in complex function, to have some helpers to 
diagnose bug reports ... if you debug a problem and you think it would be 
handy in future to have some osync_trace() in this function to debug similar 
issues in the future more easily, don't hesistate to add one. But there is no 
requirement that we put on every single function call osync_trace(TRACE_ENTRY) 
osync_trace(TRACE_EXIT) ...

In fact too much osync_trace() calls have bad impact on performance if tracing 
is enabled ... but for that we have also helpers to temporarily disable 
tracing osync_trace_enable() osync_trace_disable() ... e.g. for (quick) loops 
which get called pretty often ...


> "osynctool_list_formats" and "osynctool_list_plugins" seem to be very
>  similar, but they behave differently in this matter.

Right, osync_list_formats should look like osync_list_formats ... if you like 
you can provide a patch for that ;)

> 
> 
> Thanks in advance for answering my questions. I promise there will be some
>  more ;)

Cool - pretty good question. Looking forward to more questions and of course 
to your patches ;)

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