On 12/20/2016 11:05 AM, Amos Jeffries wrote: > +class Config > +{ > +public: > + /// set of auth_params directives > + Auth::ConfigVector schemes; > + > + /// set of auth_schemes directives > + std::vector<Auth::SchemesConfig> schemeLists; > + > + /// the ACL list for auth_schemes directives > + acl_access *schemeAccess = nullptr;
Please prohibit copying/assignment of the new Auth::Config class objects because the above class members are not ready for that. > + /// the authenticate_cache_garbage_interval > + time_t authenticateGCInterval; > + > + /// the authenticate_ttl > + time_t authenticateTTL; > + > + /// the authenticate_ip_ttl > + time_t authenticateIpTTL; Please initialize the above members of the new Auth::Config class. I know that we currently do not have a good way of specifying "correct" defaults for all Auth::Config objects except TheConfig, but we should still initialize these members. I would do something like time_t foo = 0; // TheConfig will get the right default Also, there is no good reason to repeat "authenticate" inside the Auth namespace. We should simply use gcInterval, credentialsTtl, and ipTtl member names IMO. > + for (auto *scheme : Auth::TheConfig.schemes) { > + if (scheme->configured()) > ++rv; > + } Since configured() is cons, please add const to "scheme" as well. The above changes can be done without another review round IMO. I believe the rest of the patch is mostly renaming, code shoveling, and similar changes that I am bad at reviewing, so I did not check them closely. Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev