In article <[EMAIL PROTECTED]> you wrote:
> Ralf S. Engelschall wrote:
>> 
>> In article <[EMAIL PROTECTED]> you wrote:
>> > Ralf S. Engelschall wrote:
>> >> What makes you thinking that the settings are blown away by
>> >> SSL_use_certificate and friends? These functions already have checks like ``if
>> >> ((ssl->cert == NULL) || (ssl->cert == ssl->ctx->default_cert))'' which
>> >> prevents them from blowing away the settings, Ben.
>> 
>> > Scrambled brains. You are right.
>> 
>> > A related point: I really object to all this duplicated code, such as
>> > the above. It really ought to be wrapped up in some function somewhere.
>> > In fact, I'll bet if all the duplicated code were removed from OpenSSL
>> > it would be 50% of the size or less. As well as much easier to maintain.
>> 
>> Good suggestion. Your wish is my pleasure. I've changed my patch to use a
>> ssl_cert_renew() function which and also reduced all six occurences in
>> ssl_rsa.c of the redundant code by calls to this new function (yes, it reduces
>> the amount of code lines dramatically). Votes for the appended take 3 of my
>> patch?

> Hmmm ... I would have made two functions to do the ssl_cert_renew() -
> one for a session, one for the context, and generated the error within
> them rather than in the caller. I'm also not sure about the name
> "renew", which implies it would do something even if cert is already
> set, which it won't. I'd've used "instantiate".

Yes, ssl_cert_instantiate() is fine. I've changed it to this name.  And the
suggestion with the error code is also a good one. I've changed it this way,
too.

But two functions seems oversize to me, because the difference is just that
for the context the default is not existing, so I thought it's enough to
provide this functionality by passing a NULL as the second argument.  But
ok, when we later really want we can split this without problems because
the ssl_cert_instantiate() is not part of the public API. So we can split
it without problems on demand.

Thanks for the great feedback, Ben. The final patch is now comitted.
When there are still things you want a little bit different, feel free to
adjust it.
                                       Ralf S. Engelschall
                                       [EMAIL PROTECTED]
                                       www.engelschall.com
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to