Re: [PATCH] Enhancements for the PCSC driver

[Pavel Kankovsky]

On Tue, 25 Mar 2008, Pawel Kot wrote:

> Great! Thanks for the patch.

You are welcome.

> However I'll let Daniele review these.

No problem.

> -     if (c != OPT_FOOGLE)
> +     if (c != OPT_FOOGLE && state == NULL)
> 
> state is not initialized to be NULL. So perhaps we should initialize it?

It is initialized. A static variable without an explicit initializer is
zeroed (see paragraph 10 of 6.7.8 in C99 standard).

(The extra condition is needed to prevent repeated calls of businit(). The
second call would execute atexit(busterminate) again, and repeated calls
of busterminate() would make the program crash.)

> 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.

I do not know. I suppose it depends on your aesthetic preferences.

(The version with an immediate return might yield better machine code
because the tail recursion is more obvious and more likely to be
translated to a loop but this is not a place where such a subtle
optimization could make any difference.)

> The other thing is that entersecuritycode() should return gn_error and
> not int and you should compare here with GN_ERR_NONE.

You are probably right. Nevertheless, this type mismatch had already been
there before I touched that code: the result had always been stored into
rc declared as int and returned as an exit code of the program with the
implicit assumption that ok was represented by a zero value.

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

They work as far as I can tell. Nevertheless some bizzare constructions
like the following one are possible:
$ gnokii --config F --entersecuritycode PIN --phone P --getnetworkinfo
(--phone P is used for --getnetworkinfo but not for --entersecuritycode)

A recursive call of parse_options() was the shortest way to implement that
but I have to admit I am far from convinced it was the best way.


-- 
Pavel Kankovsky aka Peak                          / Jeremiah 9:21        \
"For death is come up into our MS Windows(tm)..." \ 21th century edition /