On Mon, 14 Feb 2005 00:11:19 +0100 (CET), Daniel Stenberg
<daniel-curl_at_haxx.se> wrote:
> I disagree. I know very little about these things, so even if the changes
> themselves might be clear, I don't know what the purpose if the whole thing is
> and I want new options documented properly when added to libcurl. (That goes
> for new options to the curl command line tool as well.)
Well, people who know about ssl engines most probly know about dynamic
engines too.
The patch is different because it tries to pay special attention to
enable even _other_ crypto modules (namely, pkcs11 modules) via
openssl engine provided by OpenSC project. There are way more
different pkcs11 modules than openssl modules, i guess...

And PKCS#11 should be pretty well known thing in crypto world.

>
> > More error checking etc could be done.
>
> So I take it you will work on an updated patch?
You bet. i made the previous one at night and didn't pay much
attention to what was going on. Actually it is totally bogus and
worked only because.... well, no comments.

Attached is a proper patch for both library and user command, that
actually does what it should do.

>
> Also, a few nits/questions on the patch:
>
> - on the mod of the SessionHandle struct: you should add new options
> that an app sets to the 'struct UserDefined' struct, not the 'struct
> DynamicStatic' one. It is important to get things like curl_easy_duphandle()
> to work, and to maintain consistency with existing code.

Well, i just did a blind and fast 'grep engine' and added necessary
things nearby (as that was the obvious place, imho). Feel free to
adjust it to whatever curl-style ways you feel appropriate. As you can
see, they are just parameters to be used when creating the engine, so
they most probly don't have to be where one keeps session information.

>
> - +#define HAVE_ENGINE_LOAD_BUILTIN_ENGINES 1
> This is clearly a mistake

I couldn't find a place where this was defined and it is needed to
make the dynamic engine appear in the first place.

>
> - I see you strcmp() the engine name for "dynamic" to see if this is
> activated. Isn't it better to rely on one of the options you
> added that is bound to be set when this is used? Like SSLENGINE_ID. If not,
> is it really sane and safe to just allocate an engine name to check for
> this?
dynamic is the ssl 'engine' that allows to load other, actual engines.
SSLENGINE_ID shall be the ID that shall be given to the new, loaded
engine.

As i said - it was much of a quick and dirty hack. Though i'd
appreciate if you took some time to adjust it (as it aint that huge)
to curlisms and apply to baseline.

cheers,
m.

-- 
Martin Paljak - consultant
martin.paljak_at_gmail.com - Gmail
http://martin.paljak.pri.ee/ - web
+372.5156495 - phone