Tracing feature in the client side

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Tracing feature in the client side

Maksim Ivanov
Hello all,

I have a couple of small suggestions regarding the tracing feature of
the PC/SC-Lite's client side library. This feature is controlled by
the "DO_TRACE" preprocessor definition.

First, it's impossible to trigger this feature using the compiler
flags: for some reason, the winscard_clnt.c file contains directive
"#undef DO_TRACE". If it were a commented "#define" directive, then
this would be more useful: the feature could be triggered both by
editing the source file and by modifying the compiler flags.

Second, there is an inconsistency between the printf format specifier
and the passed value in the trace function, that produces a warning.
E.g. with clang 3.7.0:

> pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format specifies type 'unsigned long' but the argument has type 'pthread_t'
>       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>                 direction, pthread_self(), func);
>                            ^~~~~~~~~~~~~~


Regards,
Maksim

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: Tracing feature in the client side

Ludovic Rousseau
2016-03-10 13:13 GMT+01:00 Maksim Ivanov <[hidden email]>:
Hello all,

Hello Maksim,
 

I have a couple of small suggestions regarding the tracing feature of
the PC/SC-Lite's client side library. This feature is controlled by
the "DO_TRACE" preprocessor definition.

First, it's impossible to trigger this feature using the compiler
flags: for some reason, the winscard_clnt.c file contains directive
"#undef DO_TRACE". If it were a commented "#define" directive, then
this would be more useful: the feature could be triggered both by
editing the source file and by modifying the compiler flags.

Fixed in ea0a50bf2088906d888cf1888b4439f3391eb111


Second, there is an inconsistency between the printf format specifier
and the passed value in the trace function, that produces a warning.
E.g. with clang 3.7.0:

> pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format specifies type 'unsigned long' but the argument has type 'pthread_t'
>       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>                 direction, pthread_self(), func);
>                            ^~~~~~~~~~~~~~

Can you propose a patch for that?

pthread_t may not be a numerical value. It may be a structure.
http://stackoverflow.com/questions/1759794/how-to-print-pthread-t

Regards,

--
 Dr. Ludovic Rousseau

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: Tracing feature in the client side

Maksim Ivanov
Hello Ludovic,

>>
>> Second, there is an inconsistency between the printf format specifier
>> and the passed value in the trace function, that produces a warning.
>> E.g. with clang 3.7.0:
>>
>> > pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format specifies type 'unsigned long' but the argument has type 'pthread_t'
>> >       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>> >                 direction, pthread_self(), func);
>> >                            ^~~~~~~~~~~~~~
>
>
> Can you propose a patch for that?
>
> pthread_t may not be a numerical value. It may be a structure.
> http://stackoverflow.com/questions/1759794/how-to-print-pthread-t

I could propose a patch, but in that case we would have to bother with
licensing (at least, Google Inc. will have to be included into the
copyright authors list).

Anyway, I don't have a very nice solution for this problem too.
Simple, but theoretically non-portable, way would be to cast pthread_t
to 64-bit integer type (for example, that's how it's done in Chromium
- as the last-resort solution on the platforms without more useful
mechanisms: <https://cs.chromium.org/#chromium/src/base/threading/platform_thread_posix.cc&l=128>
).
Portable, but tedious, way would be to write a function that dumps the
passed data byte-by-byte.

IMO, the simplest solution with casting may be enough, as this anyway
applies to the code that is disabled by default.


Regards,
Maksim


On Fri, Mar 11, 2016 at 10:59 AM, Ludovic Rousseau
<[hidden email]> wrote:

>
> 2016-03-10 13:13 GMT+01:00 Maksim Ivanov <[hidden email]>:
>>
>> Hello all,
>
>
> Hello Maksim,
>
>>
>>
>> I have a couple of small suggestions regarding the tracing feature of
>> the PC/SC-Lite's client side library. This feature is controlled by
>> the "DO_TRACE" preprocessor definition.
>>
>> First, it's impossible to trigger this feature using the compiler
>> flags: for some reason, the winscard_clnt.c file contains directive
>> "#undef DO_TRACE". If it were a commented "#define" directive, then
>> this would be more useful: the feature could be triggered both by
>> editing the source file and by modifying the compiler flags.
>
>
> Fixed in ea0a50bf2088906d888cf1888b4439f3391eb111
>
>>
>> Second, there is an inconsistency between the printf format specifier
>> and the passed value in the trace function, that produces a warning.
>> E.g. with clang 3.7.0:
>>
>> > pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format specifies type 'unsigned long' but the argument has type 'pthread_t'
>> >       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>> >                 direction, pthread_self(), func);
>> >                            ^~~~~~~~~~~~~~
>
>
> Can you propose a patch for that?
>
> pthread_t may not be a numerical value. It may be a structure.
> http://stackoverflow.com/questions/1759794/how-to-print-pthread-t
>
> Regards,
>
> --
>  Dr. Ludovic Rousseau
>
> _______________________________________________
> Pcsclite-muscle mailing list
> [hidden email]
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle



Maksim Ivanov

Software Engineer

[hidden email]
+49 (0)176 65889070

Google Germany GmbH

Dienerstraße 12

80331 München


Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg


Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat
sind, leiten Sie diese bitte nicht weiter, informieren Sie den
Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.



This e-mail is confidential. If you are not the right addressee please
do not forward it, please inform the sender, and please erase this
e-mail including any attachments. Thanks.

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: Tracing feature in the client side

Ludovic Rousseau


2016-03-11 13:49 GMT+01:00 Maksim Ivanov <[hidden email]>:
Hello Ludovic,

>>
>> Second, there is an inconsistency between the printf format specifier
>> and the passed value in the trace function, that produces a warning.
>> E.g. with clang 3.7.0:
>>
>> > pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format specifies type 'unsigned long' but the argument has type 'pthread_t'
>> >       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>> >                 direction, pthread_self(), func);
>> >                            ^~~~~~~~~~~~~~
>
>
> Can you propose a patch for that?
>
> pthread_t may not be a numerical value. It may be a structure.
> http://stackoverflow.com/questions/1759794/how-to-print-pthread-t

I could propose a patch, but in that case we would have to bother with
licensing (at least, Google Inc. will have to be included into the
copyright authors list).

:-)
 
Anyway, I don't have a very nice solution for this problem too.
Simple, but theoretically non-portable, way would be to cast pthread_t
to 64-bit integer type (for example, that's how it's done in Chromium
- as the last-resort solution on the platforms without more useful
mechanisms: <https://cs.chromium.org/#chromium/src/base/threading/platform_thread_posix.cc&l=128>
).

That is also what I had in mind as an easy solution.
 
Portable, but tedious, way would be to write a function that dumps the
passed data byte-by-byte.

IMO, the simplest solution with casting may be enough, as this anyway
applies to the code that is disabled by default.

What version of clang do you use to get this warning?
I used "Debian clang version 3.6.2-3" with -Wformat but did not get this warning.

Bye

--
 Dr. Ludovic Rousseau

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: Tracing feature in the client side

Maksim Ivanov
> What version of clang do you use to get this warning?
> I used "Debian clang version 3.6.2-3" with -Wformat but did not get this
> warning.

As I wrote in the first post, I was using clang 3.7.0. (This is the
compiler currently used in Chrome's Native Client SDK.)


On Fri, Mar 11, 2016 at 2:01 PM, Ludovic Rousseau
<[hidden email]> wrote:

>
>
> 2016-03-11 13:49 GMT+01:00 Maksim Ivanov <[hidden email]>:
>>
>> Hello Ludovic,
>>
>> >>
>> >> Second, there is an inconsistency between the printf format specifier
>> >> and the passed value in the trace function, that produces a warning.
>> >> E.g. with clang 3.7.0:
>> >>
>> >> > pcsc-lite/src-1.8.15/src/winscard_clnt.c:162:14: error: format
>> >> > specifies type 'unsigned long' but the argument has type 'pthread_t'
>> >> >       (aka 'struct __nc_basic_thread_data *') [-Werror,-Wformat]
>> >> >                 direction, pthread_self(), func);
>> >> >                            ^~~~~~~~~~~~~~
>> >
>> >
>> > Can you propose a patch for that?
>> >
>> > pthread_t may not be a numerical value. It may be a structure.
>> > http://stackoverflow.com/questions/1759794/how-to-print-pthread-t
>>
>> I could propose a patch, but in that case we would have to bother with
>> licensing (at least, Google Inc. will have to be included into the
>> copyright authors list).
>
>
> :-)
>
>>
>> Anyway, I don't have a very nice solution for this problem too.
>> Simple, but theoretically non-portable, way would be to cast pthread_t
>> to 64-bit integer type (for example, that's how it's done in Chromium
>> - as the last-resort solution on the platforms without more useful
>> mechanisms:
>> <https://cs.chromium.org/#chromium/src/base/threading/platform_thread_posix.cc&l=128>
>> ).
>
>
> That is also what I had in mind as an easy solution.
>
>>
>> Portable, but tedious, way would be to write a function that dumps the
>> passed data byte-by-byte.
>>
>> IMO, the simplest solution with casting may be enough, as this anyway
>> applies to the code that is disabled by default.
>
>
> What version of clang do you use to get this warning?
> I used "Debian clang version 3.6.2-3" with -Wformat but did not get this
> warning.
>
> Bye
>
> --
>  Dr. Ludovic Rousseau
>
> _______________________________________________
> Pcsclite-muscle mailing list
> [hidden email]
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle