'return' versus 'SC_FUNC_RETURN'

[Viktor TARASOV-2]

Hi,

my general question is:
will you agree to have a little bit more of the debug messages in the
common sources ?
IMHO, it will facilitate the debugging and the comprehension of the 'how
it works'.

I mean, at least, to use more of the log macros ( SC_FUNC_RETURN,
SC_FUNC_CALLED and SC_TEST_RET)
instead of simple 'return SC_xxx',
to use the 'return' macro in the functions that starts from 'call'
macro, etc.

(Sometimes, in the sources, there is an textual duplicate of the
existing macros).

Does it exists any rule for the assigning of the debug level for debug
messages ?

Kind wishes,
Viktor Tarasov.

--
Viktor Tarasov <viktor.tarasov@...>

_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Aleksey Samsonov]

Hello,

Viktor TARASOV wrote:
> Hi,
>
> my general question is:
> will you agree to have a little bit more of the debug messages in the
> common sources ?
> IMHO, it will facilitate the debugging and the comprehension of the 'how
> it works'.

If it is useful, yes.


> I mean, at least, to use more of the log macros ( SC_FUNC_RETURN,
> SC_FUNC_CALLED and SC_TEST_RET)
> instead of simple 'return SC_xxx',
> to use the 'return' macro in the functions that starts from 'call'
> macro, etc.


Using everywhere these log macros is not reasonable.
It can make the simple code too difficult, overloaded with details.
But if the function is big and with complicated logic, additional
detailed log will be useful. True criterion is common sence.


> (Sometimes, in the sources, there is an textual duplicate of the
> existing macros).

What do you want to change and where?


> Does it exists any rule for the assigning of the debug level for debug
> messages ?

I think we have to follow common sence.
_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Aleksey Samsonov]

Aleksey Samsonov wrote:
>> Does it exists any rule for the assigning of the debug level for debug
>> messages ?
>
> I think we have to follow common sence.
>

Also you can find some information here
http://www.opensc-project.org/pipermail/opensc-devel/2009-September/012466.html
_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Viktor TARASOV-2]

Aleksey Samsonov wrote:

> Viktor TARASOV wrote:
>  
>> my general question is:
>> will you agree to have a little bit more of the debug messages in the
>> common sources ?
>> IMHO, it will facilitate the debugging and the comprehension of the 'how
>> it works'.
>>    
>
> If it is useful, yes.
>
>  
>> I mean, at least, to use more of the log macros ( SC_FUNC_RETURN,
>> SC_FUNC_CALLED and SC_TEST_RET)
>> instead of simple 'return SC_xxx',
>> to use the 'return' macro in the functions that starts from 'call'
>> macro, etc.
>>    
>
>
> Using everywhere these log macros is not reasonable.
> It can make the simple code too difficult, overloaded with details.
> But if the function is big and with complicated logic, additional
> detailed log will be useful. True criterion is common sence.
>  

I see, thank you, I will try to trust my common sense.

>> (Sometimes, in the sources, there is an textual duplicate of the
>> existing macros).
>>    
>
> What do you want to change and where?
>  

In a fact, I was looking through libopensc/pkcs.c (ex. +365, +525, ..) .

>> Does it exists any rule for the assigning of the debug level for debug
>> messages ?
>>    
>
> I think we have to follow common sence.
>  

In the same direction, some features that we are locally using:

Duplicate the 'sc_error' messages as the 'sc_debug' ones with some low
level of importance.
Sometimes it not evident to find the correspondence .

Message time stamp (1 msec resolution for Windows. For Linux it's 1 sec
-- was not able to do better in a simple manner) .

In the reader logs (for ex. in pcsc_transmit() of reader-pcsc.c) insert
the reader's name . It helps.

Kind wishes,
Viktor Tarasov.


> _______________________________________________
> opensc-devel mailing list
> opensc-devel@...
> http://www.opensc-project.org/mailman/listinfo/opensc-devel
>
>  


--
Viktor Tarasov <viktor.tarasov@...>

_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Viktor TARASOV-2]

Viktor TARASOV wrote:

....
> In a fact, I was looking through libopensc/pkcs.c (ex. +365, +525, ..) .
>  
Sorry, should be read as
In a fact, I was looking through libopensc/pkcs15.c (ex. +365, +525, ..) .

....


Regards,
Viktor Tarasov.

>  
>> _______________________________________________
>> opensc-devel mailing list
>> opensc-devel@...
>> http://www.opensc-project.org/mailman/listinfo/opensc-devel
>>
>>  
>>    
>
>
>  

--
Viktor Tarasov <viktor.tarasov@...>

_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Andreas Jellinghaus-2]

hmm, debugging is a problem in opensc code. here are my ideas.

I haven't figured out how to ltrace opensc, mostly when it used via opensc-
pkcs11.so. so what can I do to find a bug?
a) gdb and single step, taking notes till I find out how opensc works in
   some area and get a clue where the bug is.
b) ask for a log file and see if I can figure out what is going wrong.

but a) doesn't help if other people have a problem, I would need to
reproce the problem first, and thus very few people can debug a problem
(those with the same OS, the same app, the same card...).

b) also doesn't work great. some parts are far to noisy (card matching
works well, but generates hundreds of lines of debug code). and
asn.1 analysis might be too chatty too, unless of course the problem
is wit the format of some card - missing or extra entries etc.

also not every function has good debug code, e.g. the parameters
aren't in the debug file, so I have little clue what is going on.

having no debug code/files is not an option, I guess.
so the best we can do is to go through functions, and make
sure each and every core function prints debug info when
entered (including the parameters), and prints the result
when it is leaving. extra debug in the middle, if it is a
complex function is welcome too.

so what is useful of the code in src/libopensc/log.h?
sc_error should and everything related to error fd needs
to go - it caused debug log files without the error information,
a very bad idea. (IIRC martin did that already?)

every debug macro or function call should contain the debug
level, and the macro/function should check it against ctx->debug.
all log.h functions get the context already.

SC_FUNC_CALLED
should go away - functions have parameters, those help in debugging
a lot.

SC_FUNC_RETURN
doesn't tell us what is returned (unless the function returns only
the value).


so if we change the code, can we find better macros / ways to
debug problems? __FILE__ and __LINE__ are not very important
to me, at least it is easy to find out where a function begins,
even without that. but parameters or some detailed info
what went wrong, that all could help.

> Does it exists any rule for the assigning of the debug level for debug
> messages ?

lets put everything at 1, isolate whole function blocks that are too
chatty, and put them on a different level. (asn.1=2, card matching=3?).
we could also go for a flag based system, if that is preferable.

also (thought I guess it is already gone): suppressing errors isn't
fine with me. without debugging active, opensc shouldn't print
anything. that is what return codes are for, maybe the app can
handle it. not printing stuff when debugging is on might leave
out important parts too.

what do you think?

Regards, Andreas
_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Viktor TARASOV-2]

Hi,

Andreas Jellinghaus wrote:

> hmm, debugging is a problem in opensc code. here are my ideas.
>
> I haven't figured out how to ltrace opensc, mostly when it used via opensc-
> pkcs11.so. so what can I do to find a bug?
> a) gdb and single step, taking notes till I find out how opensc works in
>    some area and get a clue where the bug is.
> b) ask for a log file and see if I can figure out what is going wrong.
>
> but a) doesn't help if other people have a problem, I would need to
> reproce the problem first, and thus very few people can debug a problem
> (those with the same OS, the same app, the same card...).
>  

yes, sometimes the client environment is too complex to be able to
reproduce it locally.
> b) also doesn't work great. some parts are far to noisy (card matching
> works well, but generates hundreds of lines of debug code). and
> asn.1 analysis might be too chatty too, unless of course the problem
> is wit the format of some card - missing or extra entries etc.
>  
Sometimes, debug file is the last hope, (like a telemetry data from the
distant planet mission -- sorry, it's personal). And there, as for me,
it's better 'more_then_enough' then 'less_then_enough'.

> also not every function has good debug code, e.g. the parameters
> aren't in the debug file, so I have little clue what is going on.
>
> having no debug code/files is not an option, I guess.
> so the best we can do is to go through functions, and make
> sure each and every core function prints debug info when
> entered (including the parameters), and prints the result
> when it is leaving. extra debug in the middle, if it is a
> complex function is welcome too.
>  

Agree -- 'Folks! More of the debug messages, good and various!'.

> so what is useful of the code in src/libopensc/log.h?
> sc_error should and everything related to error fd needs
> to go - it caused debug log files without the error information,
> a very bad idea. (IIRC martin did that already?)
>  
> every debug macro or function call should contain the debug
> level, and the macro/function should check it against ctx->debug.
> all log.h functions get the context already.
>  
Agree for the functions.
As for macros -- I don't know how to 'MACROnize' the variable arguments
list for Windows. Does there any way to do it?

> SC_FUNC_CALLED
> should go away - functions have parameters, those help in debugging
> a lot.
>
> SC_FUNC_RETURN
> doesn't tell us what is returned (unless the function returns only
> the value).
>  

Agree.
For me, in a 'fresh' or complex code these macros are immediately
followed (preceded) by some sc_debug() with the function arguments,
object attributes, etc ... (or return code).
These macros can be useful for the short procedures, their advantage is
in the '__FILE__' and '__LINE__' that works on Windows. But with
sufficient number of the other logs these two macros can go in peace .

It not concerns the SC_TEST_RET. (Or a contrary: in continuation of your
thought above, replace it by the sc_error() function with an error
checking inside. The advantage is (once more) variable argument list on
Windows and no explicit error checking in code. ) .

> so if we change the code, can we find better macros / ways to
> debug problems? __FILE__ and __LINE__ are not very important
> to me, at least it is easy to find out where a function begins,
> even without that. but parameters or some detailed info
> what went wrong, that all could help.
>  
>> Does it exists any rule for the assigning of the debug level for debug
>> messages ?
>>    
>
> lets put everything at 1, isolate whole function blocks that are too
> chatty, and put them on a different level. (asn.1=2, card matching=3?).
> we could also go for a flag based system, if that is preferable.
>  

... and the last one is for the reader status functions called by
pcsc/winscard daemon (my five kopeckes).
Flags system is quiet attractive.

> also (thought I guess it is already gone): suppressing errors isn't
> fine with me. without debugging active, opensc shouldn't print
> anything.
No opinion. For me the opensc-error.log is like debug file with level
'0' -- user can send it to us immediately.
(In general it doesn't helps, and he is asked to activate all debugs .)
But, why not.


> that is what return codes are for, maybe the app can
> handle it. not printing stuff when debugging is on might leave
> out important parts too.
>  
Why not.
> what do you think?
>  
- should be more of the debug messages;
- in the code the explicit debug level checking looks not very nice;
- probably the same about explicit error checking;
- MACROs can be replaced the the error/debug functions with the
error/debug_level checking inside.

Do you think that it's reasonable to add some time stamp into the debug
messages?
It's quiet useful in the complex environment.
The same about the reader's name in the debug messages at the
pcsc_transmit level.

> Regards, Andreas
>  
Kind wishes,
Viktor.

--
Viktor Tarasov <viktor.tarasov@...>

_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Peter Stuge-4]

Andreas Jellinghaus wrote:
> > Does it exists any rule for the assigning of the debug level for debug
> > messages ?
>
> lets put everything at 1, isolate whole function blocks that are too
> chatty, and put them on a different level. (asn.1=2, card matching=3?).
> we could also go for a flag based system, if that is preferable.

Flag based, or maybe "function block" based, seems like a nice idea.


//Peter
_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Martin Paljak-2]

Hi Viktor
On 04.11.2009, at 14:52, Viktor TARASOV wrote:

> Message time stamp (1 msec resolution for Windows. For Linux it's 1 sec
> -- was not able to do better in a simple manner) .
>
> In the reader logs (for ex. in pcsc_transmit() of reader-pcsc.c) insert
> the reader's name . It helps.


Do you have a patch with these changes or it is just a suggestion?



--
Martin Paljak
http://martin.paljak.pri.ee
+372.515.6495




_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Viktor TARASOV-2]

Martin Paljak wrote:

> Hi Viktor
> On 04.11.2009, at 14:52, Viktor TARASOV wrote:
>
>  
>> Message time stamp (1 msec resolution for Windows. For Linux it's 1 sec
>> -- was not able to do better in a simple manner) .
>>
>> In the reader logs (for ex. in pcsc_transmit() of reader-pcsc.c) insert
>> the reader's name . It helps.
>>    
>
>
> Do you have a patch with these changes or it is just a suggestion?
>  

Yes, it's implemented in our local version.
If no objection, I'll submit it to trunk.

--
Viktor Tarasov <viktor.tarasov@...>

_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel

[Martin Paljak-2]

On 13.11.2009, at 16:42, Viktor TARASOV wrote:

>>> Message time stamp (1 msec resolution for Windows. For Linux it's 1 sec
>>> -- was not able to do better in a simple manner) .
>>>
>>> In the reader logs (for ex. in pcsc_transmit() of reader-pcsc.c) insert
>>> the reader's name . It helps.
>>>
>>
>>
>> Do you have a patch with these changes or it is just a suggestion?
>>
>
> Yes, it's implemented in our local version.
> If no objection, I'll submit it to trunk.

Feels useful, OK by me. We should be moving the 0.12 branch as trunk, but...


--
Martin Paljak
http://martin.paljak.pri.ee
+372.515.6495




_______________________________________________
opensc-devel mailing list
opensc-devel@...
http://www.opensc-project.org/mailman/listinfo/opensc-devel