On 13/07/2012 5:30 a.m., Tsantilas Christos wrote:
This is a new version of the patch.
Not all of Amos comments handled please see my comments bellow.


On 07/11/2012 03:38 PM, Amos Jeffries wrote:
src/cf.data.pre:
* Please use "IF " instead of "IFDEF " for the new inline #if macro. It
is confusing to tell whether any particular one should be IFDEF<space>
or IFDEF<colon>.
I made this change but I have my doubts.
The IFDEF is a reserved word which already used by cf_gen parser so it
doesn't hurt to use it in a new form. The "if" is a valid statement of
squid configuration file.

The other form though is a very different meaning - the entire directive block of details is elided and replaced with instructions on what to rebuild Squid with. If this form were interchanged with the other form we would end up with whole block sections of unrelated code missing. We *really* don't want these two to get confused in the future misunderstanding.


src/client_side.h:
* setServerBump(Ssl::ServerBump *srvBump) has no else condition
reporting or handling when setting fails.
It should not called if the bump mode already decided. Also we have
other debug messages in other places to inform users for this
bug/problem. To print the message in setServerBump we should convert
this method to a non inline method. Does it worth?

Not worth it then. Can we assert(sslServerBump == srvBump) to catch future abuse?

Ack, on the rest about pinning.


Amos

Reply via email to