On 10/31/2013 02:47 PM, Amos Jeffries wrote: > On 31/10/2013 1:16 p.m., Alex Rousskov wrote: >> Hello, >> >> The attached patch upgrades Squid to libecap v1.0, allowing >> asynchronous (e.g., threaded) eCAP adapters and eCAP version checks. >> >> After these changes, Squid can support eCAP adapters built with libecap >> v1.0, but stops supporting adapters built with earlier libecap versions >> (due to libecap API changes). The new libecap version allows Squid to >> better check the version of the eCAP adapter being loaded as well as the >> version of the eCAP library being used. This should help with migration >> to libecap v1.0. >> >> >> Thank you, >> >> Alex. > > In src/adaptation/ecap/Host.cc: > * since both parameters passed to SupportedVersion() start out as char*. > Please consider using SBuf instead of std::string and char* manipulations.
> - otherwise please at least use std::string() constructor to make the > conversion explicit. It looks quite strange to have auto-conversion from > char* to std::string& on the second parameter, but strangely not on the > first parameter. Char* type for the first argument is necessary because that argument may be NULL (rather than empty) in legacy adapter cases. Both arguments are using char* type now (with an explicit conversion to SBuf) to address your "consistency" concerns. > Other than the above the code appears consistent. The above can be done > without needing abother round of review IMO. > So I'm giving this a +0 (neutral about it going in as-is, the SBuf usage > will make it +1). Committed to trunk (r13155) after converting std::string to SBuf as requested above and fixing "make check" dependencies. Thank you, Alex.
