Polished to address Squid Project review comments.

diff --git a/configure.ac b/configure.ac
index e9105768f3933d524d80e94f697f5f29ecc8a4cd..0289c3f6f414594aede6cebcc0e8c606678702de 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -913,10 +913,10 @@ then
         AC_MSG_ERROR([eCAP support requires loadable modules. Please do not use --disable-loadable-modules with --enable-ecap.]);
     fi
 
-    dnl eCAP support requires libecap
-    dnl This Squid supports libecap v0.2.x
-    dnl Use EXT prefix or libtool will get confused with two libecaps:
-    dnl one "external" (that we check for here) and one our own convenience lib
+    dnl eCAP support requires libecap.
+    dnl This Squid supports libecap v0.2.x.
+    dnl Use EXT prefix so that make and libtool messages distinguish between
+    dnl external libecap (that we check for here) and our own convenience lib.
     PKG_CHECK_MODULES(EXTLIBECAP, [libecap > 0.2 libecap < 0.3])
 fi
 

diff --git a/src/Server.cc b/src/Server.cc
index c1e47de2927ce1f42ccbaffa1ae0c04ce712a12a..cbf2d39c566c0d4caaed58d825a1a72ef4a584a3 100644 (file)
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -793,7 +793,7 @@ ServerStateData::handleAdaptationAborted(bool bypassable)
 void
 ServerStateData::handleAdaptationBlocked(const Adaptation::Answer &answer)
 {
-    debugs(11,5, HERE << "handleAdaptationBlocked: " << answer.ruleId);
+    debugs(11,5, HERE << answer.ruleId);
 
     if (abortOnBadEntry("entry went bad while ICAP aborted"))
         return;

diff --git a/src/adaptation/Config.h b/src/adaptation/Config.h
index f9f8dfd8d1a0e53f9a34d84f53230034013244dd..2740f9808124a5a2965076970689ca224ab8df28 100644 (file)
--- a/src/adaptation/Config.h
+++ b/src/adaptation/Config.h
@@ -64,7 +64,7 @@ private:
     Config(const Config &); // unsupported
     Config &operator =(const Config &); // unsupported
 
-    virtual ServicePointer createService(ServiceConfigPointer cfg) = 0;
+    virtual ServicePointer createService(const ServiceConfigPointer &cfg) = 0;
 
     static void ParseServiceGroup(ServiceGroupPointer group);
     static void FreeServiceGroups(void);

diff --git a/src/adaptation/Makefile.am b/src/adaptation/Makefile.am
index 7fd6401634f2c4ff4ca0d1dda3a2c698952667d0..0a1d39909c29d7600232c74bb57e46c4de006517 100644 (file)
--- a/src/adaptation/Makefile.am
+++ b/src/adaptation/Makefile.am
@@ -45,7 +45,6 @@ libadaptation_la_SOURCES = \
        History.h 
 
 # add libraries for specific adaptation schemes
-libadaptation_la_LDFLAGS = $(EXTLIBECAP_LIBS)
-libadaptation_la_LIBADD = $(ECAP_LIBS) $(ICAP_LIBS)
+libadaptation_la_LIBADD = $(EXTLIBECAP_LIBS) $(ECAP_LIBS) $(ICAP_LIBS)
 
 libadaptation_la_DEPENDENCIES = $(ECAP_LIBS) $(ICAP_LIBS)

diff --git a/src/adaptation/Service.cc b/src/adaptation/Service.cc
index 9f446361df5ea8d2fb3b36a895a19a505096dc34..dd297d8a91ae0682b23dfcc198440ba511502813 100644 (file)
--- a/src/adaptation/Service.cc
+++ b/src/adaptation/Service.cc
@@ -7,7 +7,7 @@
 #include "adaptation/ServiceFilter.h"
 #include "adaptation/Service.h"
 
-Adaptation::Service::Service(ServiceConfigPointer aConfig): theConfig(aConfig)
+Adaptation::Service::Service(const ServiceConfigPointer &aConfig): theConfig(aConfig)
 {
     Must(theConfig != NULL);
     debugs(93,3, HERE << "creating adaptation service " << cfg().key);

diff --git a/src/adaptation/Service.h b/src/adaptation/Service.h
index 3b9f47b0a0b58a8122f760e936a7aef4f8f01312..6679a7f8a493c5d17dd14348b6192daf6d5d6b73 100644 (file)
--- a/src/adaptation/Service.h
+++ b/src/adaptation/Service.h
@@ -24,7 +24,7 @@ public:
     typedef String Id;
 
 public:
-    explicit Service(ServiceConfigPointer aConfig);
+    explicit Service(const ServiceConfigPointer &aConfig);
     virtual ~Service();
 
     virtual bool probed() const = 0; // see comments above

diff --git a/src/adaptation/ServiceConfig.cc b/src/adaptation/ServiceConfig.cc
index 219df67e2bf5fe627f1cca336f4cc556e560142f..44f1a8b43618c286207bc80b95cc620be4c4922d 100644 (file)
--- a/src/adaptation/ServiceConfig.cc
+++ b/src/adaptation/ServiceConfig.cc
@@ -250,7 +250,8 @@ bool
 Adaptation::ServiceConfig::grokExtension(const char *name, const char *value)
 {
     // we do not accept extensions by default
-    debugs(3, 0, cfg_filename << ':' << config_lineno << ": " <<
-           "unknown adaptation service option: " << name << '=' << value);
+    debugs(3, DBG_CRITICAL, cfg_filename << ':' << config_lineno << ": " <<
+           "ERROR: unknown adaptation service option: " <<
+           name << '=' << value);
     return false;
 }

diff --git a/src/adaptation/ecap/Config.cc b/src/adaptation/ecap/Config.cc
index dcdcc11d4b535c1b901f61ee148f34b54e1f60aa..c9eedb2093f4c0bb1700c61b508ba4fa76ce0cf8 100644 (file)
--- a/src/adaptation/ecap/Config.cc
+++ b/src/adaptation/ecap/Config.cc
@@ -33,7 +33,7 @@ Adaptation::Ecap::Config::newServiceConfig() const
 }
 
 Adaptation::ServicePointer
-Adaptation::Ecap::Config::createService(ServiceConfigPointer cfg)
+Adaptation::Ecap::Config::createService(const ServiceConfigPointer &cfg)
 {
     return new Adaptation::Ecap::ServiceRep(cfg);
 }

diff --git a/src/adaptation/ecap/Config.h b/src/adaptation/ecap/Config.h
index b9171db34722c84d094bb1f8c4863de3cf8fa572..79d65c7ee91093b0104138997e4d5afe71aaee8c 100644 (file)
--- a/src/adaptation/ecap/Config.h
+++ b/src/adaptation/ecap/Config.h
@@ -46,7 +46,7 @@ private:
     Config(const Config &); // not implemented
     Config &operator =(const Config &); // not implemented
 
-    virtual Adaptation::ServicePointer createService(ServiceConfigPointer cfg);
+    virtual Adaptation::ServicePointer createService(const ServiceConfigPointer &cfg);
 };
 
 extern Config TheConfig;

diff --git a/src/adaptation/ecap/Makefile.am b/src/adaptation/ecap/Makefile.am
index eff0af1334e76fc10ab594f71a5a8f5552c983d3..4e414e68524e3e2c97e4fbe29c96d5d5e58988a4 100644 (file)
--- a/src/adaptation/ecap/Makefile.am
+++ b/src/adaptation/ecap/Makefile.am
@@ -18,9 +18,9 @@ libxecap_la_SOURCES = \
        Registry.h
 
 # add libecap using its pkg-config-produced configuration variables
-libxecap_la_CPPFLAGS = $(EXTLIBECAP_CFLAGS)
+libxecap_la_CXXFLAGS = $(EXTLIBECAP_CFLAGS)
 
-## It is tempting to put libxecap_la_LDFLAGS here, but it leads to weird
+## It is tempting to put libxecap_la_LDFLAGS/LIBADD here, but it leads to weird
 ## linking errors. For example, "make clean all" works, but rebuilding after
 ## modifying a single source file leads to libtool's "file not found" errors.
-## libxecap_la_LDFLAGS = $(EXTLIBECAP_LIBS)
+## libxecap_la_LIBADD = $(EXTLIBECAP_LIBS)

diff --git a/src/adaptation/ecap/ServiceRep.cc b/src/adaptation/ecap/ServiceRep.cc
index c22d060bb41cc6d6dbbc5b22993e1de2a521d826..16e7530dcba13be2f075f151c90212b7995f3450 100644 (file)
--- a/src/adaptation/ecap/ServiceRep.cc
+++ b/src/adaptation/ecap/ServiceRep.cc
@@ -78,7 +78,7 @@ Adaptation::Ecap::ConfigRep::visitEachOption(libecap::NamedValueVisitor &visitor
 
 
 
-Adaptation::Ecap::ServiceRep::ServiceRep(ServiceConfigPointer cfg):
+Adaptation::Ecap::ServiceRep::ServiceRep(const ServiceConfigPointer &cfg):
         /*AsyncJob("Adaptation::Ecap::ServiceRep"),*/ Adaptation::Service(cfg),
         isDetached(false)
 {

diff --git a/src/adaptation/ecap/ServiceRep.h b/src/adaptation/ecap/ServiceRep.h
index a531b7693da9feaed98f7f87ca05a28efc497429..4a51049ac2a6d8e5dd9fe9e3b58394d607d2dd24 100644 (file)
--- a/src/adaptation/ecap/ServiceRep.h
+++ b/src/adaptation/ecap/ServiceRep.h
@@ -23,7 +23,7 @@ namespace Ecap
 class ServiceRep : public Adaptation::Service
 {
 public:
-    ServiceRep(ServiceConfigPointer aConfig);
+    explicit ServiceRep(const ServiceConfigPointer &aConfig);
     virtual ~ServiceRep();
 
     typedef libecap::shared_ptr<libecap::adapter::Service> AdapterService;

diff --git a/src/adaptation/icap/Config.cc b/src/adaptation/icap/Config.cc
index 3f6b5dcd200d5eed4d9019448e51d7195a51f27c..79274354633f9ddb5e6001306e0c4361a2da789a 100644 (file)
--- a/src/adaptation/icap/Config.cc
+++ b/src/adaptation/icap/Config.cc
@@ -58,7 +58,7 @@ Adaptation::Icap::Config::~Config()
 }
 
 Adaptation::ServicePointer
-Adaptation::Icap::Config::createService(ServiceConfigPointer cfg)
+Adaptation::Icap::Config::createService(const ServiceConfigPointer &cfg)
 {
     return new Adaptation::Icap::ServiceRep(cfg);
 }

diff --git a/src/adaptation/icap/Config.h b/src/adaptation/icap/Config.h
index ad423c2619f271b4c8277cdb599d2cc73426e4a3..587953aab23f80bdc102b98e4442915b53ab8108 100644 (file)
--- a/src/adaptation/icap/Config.h
+++ b/src/adaptation/icap/Config.h
@@ -76,7 +76,7 @@ private:
     Config(const Config &); // not implemented
     Config &operator =(const Config &); // not implemented
 
-    virtual Adaptation::ServicePointer createService(ServiceConfigPointer cfg);
+    virtual Adaptation::ServicePointer createService(const ServiceConfigPointer &cfg);
 };
 
 extern Config TheConfig;

diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc
index 243be4d2faf89044b4a9a72ecd31b8689587a38b..a3faa9a2b19c1209e64b5ad6c19e4b275fe64be5 100644 (file)
--- a/src/adaptation/icap/ServiceRep.cc
+++ b/src/adaptation/icap/ServiceRep.cc
@@ -15,7 +15,7 @@
 
 CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, ServiceRep);
 
-Adaptation::Icap::ServiceRep::ServiceRep(ServiceConfigPointer svcCfg):
+Adaptation::Icap::ServiceRep::ServiceRep(const ServiceConfigPointer &svcCfg):
         AsyncJob("Adaptation::Icap::ServiceRep"), Adaptation::Service(svcCfg),
         theOptions(NULL), theOptionsFetcher(0), theLastUpdate(0),
         isSuspended(0), notifying(false),

diff --git a/src/adaptation/icap/ServiceRep.h b/src/adaptation/icap/ServiceRep.h
index 6039baec187d9d859b033a290adfa8143f29ef63..bbf0f9178d01c847bb664193ccb22b1c10b45ff2 100644 (file)
--- a/src/adaptation/icap/ServiceRep.h
+++ b/src/adaptation/icap/ServiceRep.h
@@ -87,7 +87,7 @@ public:
     typedef RefCount<ServiceRep> Pointer;
 
 public:
-    ServiceRep(ServiceConfigPointer aConfig);
+    explicit ServiceRep(const ServiceConfigPointer &aConfig);
     virtual ~ServiceRep();
 
     virtual void finalize();

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 2baeeea42c1bfe8766df543eb6c44b0af6f10ea3..f16cd634cc4980d51f81f810933106fbad7a43dd 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -6758,10 +6758,12 @@ DOC_START
 
        An ICAP REQMOD or RESPMOD transaction may set an entry in the 
        shared table by returning an ICAP header field with a name 
-       specified in adaptation_masterx_shared_names. An eCAP REQMOD or
-       RESPMOD transaction may set an entry in the shared table by
-       implementing the libecap::visitEachOption() API to provide an
-       option with a name specified in adaptation_masterx_shared_names.
+       specified in adaptation_masterx_shared_names.
+
+       An eCAP REQMOD or RESPMOD transaction may set an entry in the
+       shared table by implementing the libecap::visitEachOption() API
+       to provide an option with a name specified in
+       adaptation_masterx_shared_names.
 
        Squid will store and forward the set entry to subsequent adaptation
        transactions within the same master transaction scope.

diff --git a/src/cf_gen_defines b/src/cf_gen_defines
index aa5464ab39947ed8ce220ec10c1071d5eda2baf8..78fc510b96ce339a9fddbb250a06a1bfb1b53629 100644 (file)
--- a/src/cf_gen_defines
+++ b/src/cf_gen_defines
@@ -8,7 +8,7 @@ BEGIN {
        define["USE_DELAY_POOLS"]="--enable-delay-pools"
        define["FOLLOW_X_FORWARDED_FOR"]="--enable-follow-x-forwarded-for"
        define["FOLLOW_X_FORWARDED_FOR&&USE_DELAY_POOLS"]="--enable-follow-x-forwarded-for and --enable-delay-pools"
-       define["FOLLOW_X_FORWARDED_FOR&&ICAP_CLIENT"]="--enable-follow-x-forwarded-for and --enable-icap-client"
+       define["FOLLOW_X_FORWARDED_FOR&&USE_ADAPTATION"]="--enable-follow-x-forwarded-for and (--enable-icap-client and/or --enable-ecap)"
        define["FOLLOW_X_FORWARDED_FOR&&LINUX_NETFILTER"]="--enable-follow-x-forwarded-for and --enable-linux-netfilter"
        define["USE_HTTP_VIOLATIONS"]="--enable-http-violations"
        define["ICAP_CLIENT"]="--enable-icap-client"