Re: [PATCH] Enhancements for the PCSC driver

[Pawel Kot]

Hi,

On Mon, Mar 24, 2008 at 7:32 PM, Pavel Kankovsky
<address@hidden> wrote:
> Hello! I have added some new functions to the PCSC driver:

Great! Thanks for the patch.

>  1. GN_OP_DeleteSMS and GN_OP_DeletePhonebook make it to wipe data from
>  SIM cards (the implementation overwrites all bytes deliberately to make
>  recovery of erased data more difficult),
>
>  2. GN_OP_EnterSecurityCode and GN_OP_GetSecurityCodeStatus provide some
>  basic support for SIM cards protected with PIN (only PIN vulgo CHV1 is
>  supported and the report of status is somewhat heuristic),
>
>  3. GN_OP_GetSMS has been enhanced to return message status (read/unread
>  etc.) and handle unexpected data (e.g. wiped records) more gracefully.

However I'll let Daniele review these.

>  See the attached gnokii-0.6.24-pcsc.diff. Moreover, I enclose two
>  additional auxiliary patches I needed to make the driver useful for me:
>
>  A. gnokii-0.6.24-cmdline.diff: gnokii (the program) is difficult to use
>  with SIM cards (unless you disable PIN) because it does one command per
>  PCSC session. This hack makes it possible to couple --entersecuritycode
>  with another command, e.g.:
>  $ gnokii --entersecuritycode PIN --getnetworkinfo

Few minor things here:

-       if (c != OPT_FOOGLE)
+       if (c != OPT_FOOGLE && state == NULL)

state is not initialized to be NULL. So perhaps we should initialize it?

+               if (rc == 0 && optind < argc)
+                       return parse_options(argc, argv);

I know that similiar construct is used for config file handling, but I
wonder whether:
+               if (rc == 0 && optind < argc)
+                       rc = parse_options(argc, argv);
wouldn't be better. The other thing is that entersecuritycode() should
return gn_error and not int and you should compare here with
GN_ERR_NONE.

Did you test how this combines with --config and --phone options? (I
cannot test right now)

Otherwise this patch looks good and very useful!

>  B. gnokii-0.6.24-autoconf.diff: adds support for older versions of
>  pcsc-lite. I used Gnokii with pcsc-lite 1.3.1 (the version included in my
>  distro) and as far as I can tell, the only problem with it was the lack of
>  LPCSTR typedef (worked around by this patch).

This one looks good as well but I'll let Daniele take an action on this.

take care,
pkot
-- 
Pawel Kot