[squid-dev] [PATCH] ServerBump class cleanup

Alex Rousskov rousskov at measurement-factory.com
Wed Nov 30 14:57:26 UTC 2016


On 11/30/2016 06:34 AM, Amos Jeffries wrote:
> This patch is a general cleanup of coding styles and current code
> requirements for the ServerBump class.

> -    void attachServerSSL(SSL *); ///< Sets the server SSL object
> +    void attachServerSession(const Security::SessionPointer &); ///< Sets the server SSL object

The comment continues to lie. If you rename the method, please fix the
comment as well (and mention that the method does nothing if the session
has already been set).

Renaming attachServerSSL() and serverSSL() is the only justifiable
change IMO. The maintenance/porting headaches created by other polishing
touches are not worth it. I cannot block them, but I do not recommend
making them.


> Is there any other general cleanup that should be done to this class
> while I'm at it ?

I do not think we should be doing any general cleanup of the frequently
used class that should be completely redesigned instead.

Alex.



More information about the squid-dev mailing list