Hello, Few questions:
On Sun, Apr 17, 2011 at 12:44, Peter Marschall <[email protected]> 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 [email protected] http://www.opensc-project.org/mailman/listinfo/opensc-devel
