Am Montag 12 Oktober 2009 08:45:42 schrieb François Leblanc:
> Hi Andreas,
>
> Thank you for your comments,
>
> >things come to my mind:
> >* add a few comments where the new pcsc_card and pcsc_ctx are looked at,
> >  so people know "ah, the basecsp with pcsc connection given to us".
> >  also do you need the config file code for testing? if not, we could
> >  drop it (or is there a useful way to do that? I guess thoe variables
> >  contains a pointer, so a config file entry seems pointless to me).
>
> For pcsc_card and pcsc_ctx contains pointer it's right. Sorry I don't see
> what you mean with " do you need the config file code for testing?". I
> guess for the first point that you mean comments about where come the 
> value of pcsc_card and pcsc_ctx it is? If so you're right.

you know those new values before your initialize them.
and you need those values in the code to initialize pcsc reader driver.

currently you have a mixed implementation: the structure sc_context_param_t
gets extended to contain those new parameters.

we could extend the context structure, so we can copy the parameters in
there, and pcsc_init would have them via the normal call chain.

or we could add extra parameters thoughout the call chain, to pass them
that way.

or - like you implement - find some dynamic structure that is both available
when the context is created and later when those values are used, so they
are stored there and later retriev by pcsc_init from that location.
you used this mechanism with the config file code.

so I wonder if it that is the best option, or if we shouldn't use some
other way, maybe place the values in context structure, or extend the
call chain to pass extra parameters / extend the structures already
passed.

the call chain is:

int sc_context_create(sc_context_t **ctx_out, const sc_context_param_t *parm)
        sc_context_param already extended for the new values

static int load_reader_drivers(sc_context_t *ctx,
                               struct _sc_ctx_options *opts)
        _sc_ctx_otions could be extended too, to carry these options


        driver->ops->init(ctx, &ctx->reader_drv_data[i]);
        static int pcsc_init(sc_context_t *ctx, void **reader_data)
                has only the context and the reader_data from config file.

we could define some new structure with reader parameters to be passed
as third argument and store the values there. still a hack, but I wonder
if that would be cleaner than using the config file code as transport.

not sure, what does everyone else think?

> >* the code is hard coded to 1024bit rsa? is that a basecsp requirement?
>
> No, I probably let some reference to 1024bit somewhere is why you think
> that but is available for more than 1024bit key length.

I saw this
+       BYTE modulus[1024/8]; 
and thought it would limit the code to 1024 byte keys maybe?

> I've in mind to generate this file like it done with opensc-install.bat. I
> think it will be necessary if we want to manage all opensc card since you
> have to provide this for each atr card.

ah. can you document in README maybe what registry keys are necessary?
or maybe better a wiki page about the new baseCSP mini driver,
so we have all the information in that central place.

I guess people will want to customize / configure this, or for debugging
will need to check if some other driver claimed some atr opensc wants to
claim too.

thanks for your great work!

Regards, Andreas
_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel

Reply via email to