Hello,
Few questions:
On Sun, Apr 17, 2011 at 12:44, Peter Marschall <pe...@adpm.de> wrote:
> please find attached 3 patches to opensc-tool and opensc-explorer:
>
> * [PATCH 1/3] opensc-tool: make list_algorithms() table driven
> Use easily extensible tables instead of explicit coding to display
> algorithm names and options in list_algorithms.
>
> Leverage the new tables to add more RSA hashes
* The patch has a lot of extra whitespaces at end of lines. If you
change the line in options table for list-algorithms, you could maybe
nicely space-align the rest of the table as well ? :) If you use git,
enable the sampel "pre-commit" hook in .git/hooks to detect such
whitespace errors.
* Why the commented out "none" entries for hashes and paddings and
special handling in the loop for none? As a general rule: please don't
do c++ comments and no commented out code in new commits
> * [PATCH 2/3] opensc-tool: convert print_file() to using tables
> Use ID<->name tables in print_file() innstead of arrays of strings where
> the index was treated like some "magic" constant. With the new mapping
> tables, the meaning is obvious.
>
> While on it, fix a bug with ac_ops_df[]: before the conversion, it was a list
> of pointers to strings but was in one case treated like it was a mapping
> table.
> With the conversion to a mapping table, and the adaption of other code parts
> this bug got fixed "automagically" ;-)
OK.
> * [PATCH 3/3] opensc-{explorer,tool}: allow sending extended APDUs
> In do_apdu() resp. send_apdu/(, flexibilize parsing the APDU string passed
> so that extended APDUs are accepted a valid APDUs too.
OK. Also related is #237, at least to the extent that is described in
commend 12 [1]. As you already notices, that's copied code. Pushing it
to a single location and re-using would be a better choice.
> While at it, fix a bug where more data than available would have been copied,
> potentially leading to a SIGSEGV.
> Please consider including them into trunk, as they
> a) fix potential bugs
> b) help development: send extedned APDUs
> c) allow tools to give more complete information
Thanks!
I could fix 1 (comments on "none" handling?) and commit 2, but it
would be great if you could revise it if possible. I don't know about
3. It would be best to move it to a single location but that could be
deferred as well.
Thanks,
Martin
[1] https://www.opensc-project.org/opensc/ticket/237#comment:12
_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel
