From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Tue, 8 Mar 2011 23:56:22 +0000 (-0700)
Subject: Polished to address Squid Project review comments.
X-Git-Tag: take06~27^2~108^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6666da1185e0b2bd03bc520d5a2ab81c016f6a9a;p=thirdparty%2Fsquid.git

Polished to address Squid Project review comments.
---

diff --git a/configure.ac b/configure.ac
index e9105768f3..0289c3f6f4 100644
--- 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 c1e47de292..cbf2d39c56 100644
--- 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 f9f8dfd8d1..2740f98081 100644
--- 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 7fd6401634..0a1d39909c 100644
--- 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 9f446361df..dd297d8a91 100644
--- 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 3b9f47b0a0..6679a7f8a4 100644
--- 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 219df67e2b..44f1a8b436 100644
--- 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 dcdcc11d4b..c9eedb2093 100644
--- 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 b9171db347..79d65c7ee9 100644
--- 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 eff0af1334..4e414e6852 100644
--- 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 c22d060bb4..16e7530dcb 100644
--- 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 a531b7693d..4a51049ac2 100644
--- 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 3f6b5dcd20..7927435463 100644
--- 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 ad423c2619..587953aab2 100644
--- 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 243be4d2fa..a3faa9a2b1 100644
--- 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 6039baec18..bbf0f9178d 100644
--- 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 2baeeea42c..f16cd634cc 100644
--- 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 aa5464ab39..78fc510b96 100644
--- 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"