Re: [PATCH] cert validation cache

On 12/12/2012 03:33 AM, Amos Jeffries wrote:
> On 12.12.2012 05:02, Alex Rousskov wrote:
>> On 12/11/2012 03:50 AM, Amos Jeffries wrote:
>>> On 11/12/2012 9:19 p.m., Tsantilas Christos wrote:
>>>> If there is not any objection I will apply the latest "cert validation
>>>> cache" patch to trunk.
>>
>>> This patch is also threaded with "#if 1 // USE_SSL_CERT_VALIDATOR" just
>>> like the other one and will need re-testing without it.
>>
>> I think we should either use proper USE_SSL_CERT_VALIDATOR conditional
>> or make this code unconditional. Iff nobody has strong opinions about
>> it, I suggest making this code unconditional (no #ifs).
>>
>> The certificate validator is not enabled by default and the extra code
>> does not add a lot of performance overhead, does it?
>>
>
> Up to you guys whether the conditionals exist or not. But it definitely
> will need USE_SSL around the SSL-specific code.

I think the USE_SSL exist in all places required.
If there is not any objection:
 - I will move the LruMap under the src/base directory
 - I will apply this patch
 - I will remove with a second patch all "#if 1 //USE_SSL_CERT_VALIDATOR"

> PS. Christos, can you please avoid even developing with the "#if 1 //
> USE_BLAH" style of force-enabling. If it is worth putting the
> conditionals in at all it is worth adding a ./configure option and
> testing during development with that options set/unset.

In this project I use it as a comment. Something like "//Here starts
cert validator code" and "//here stops the cert validator code".
There are cases inside squid where we have similar blocks of code inside
"#if 1" or "#if 0".
For this project it is not worth such configure option.
I agree it may confuse other developers, so it is better to just remove
it...

>
> Amos
>
>
Received on Wed Dec 12 2012 - 14:06:31 MST