date:20160311

[squid-dev] [PATCH] g++ -Woverloaded-virtual

2016-03-11 Thread Alex Rousskov

Hello,

I accidentally discovered that Squid does not use
-Woverloaded-virtual when compiled with GCC. I found that warning useful
in other projects. Enabling it for Squid exposes one bug-in-the-making:
It looks like an 3rd Ftp::Relay::failed() argument was forgotten. The
attached patch enables the warning and probably fixes Ftp::Relay::failed().

Do we want to add this warning?

Disclaimer: The Ftp::Relay::failed() fix is correct from removing the
warning point of view, but I have not checked whether the patched code
handles non-nil ftpErr correctly. AFAICT, this method is never called
with non-nil ftpErr today, but that may change.


Thank you,

Alex.
P.S. Here is GCC's documentation for this warning, but I think its
primary value in Squid context is in detecting unintended, hidden
changes in method profiles, not avoiding compilation errors:

> -Woverloaded-virtual (C++ and Objective-C++ only)
> Warn when a function declaration hides virtual functions from a
> base class.  For example, in:
> 
> struct A {
>   virtual void f();
> };
> 
> struct B: public A {
>   void f(int);
> };
> 
> the "A" class version of "f" is hidden in "B", and code like:
> 
> B* b;
> b->f();
> 
> fails to compile.
Detect when a child method declaration hides parent virtual method.

Adding -Woverloaded-virtual exposed one problem in existing code.

=== modified file 'configure.ac'
--- configure.ac	2016-02-25 16:49:08 +
+++ configure.ac	2016-03-11 22:20:51 +
@@ -328,41 +328,41 @@ if test "x$PRESET_CFLAGS" = "x"; then
 *)
   ;;
 esac
   fi
 fi
 
 dnl set squid required flags
 if test "$squid_cv_compiler" = "gcc"; then
   case "$squid_host_os" in
   mingw)
 dnl Guido Serassio (seras...@squid-cache.org) 20070811
 dnl Using the latest MinGW (gcc 3.4.5 + mingw-runtime 3.13) cannot build with
 dnl -Werror -Wmissing-prototypes -Wmissing-declarations
 dnl TODO: check if the problem will be present in any other newer MinGW release.
 SQUID_CFLAGS="$squid_cv_cc_option_wall -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow"
 ;;
   *)
 SQUID_CFLAGS="$squid_cv_cc_option_wall -Wpointer-arith -Wwrite-strings -Wmissing-prototypes -Wmissing-declarations -Wcomments -Wshadow"
 ;;
   esac
-  SQUID_CXXFLAGS="$squid_cv_cc_option_wall -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow"
+  SQUID_CXXFLAGS="$squid_cv_cc_option_wall -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow -Woverloaded-virtual"
 else
   SQUID_CFLAGS=
   SQUID_CXXFLAGS=
 fi
 
 dnl CentOS (and RHEL) still define ntohs() using deprecated C++ features
 SQUID_CC_REQUIRE_ARGUMENT([ac_cv_require_wno_deprecated_register],[-Werror -Wno-deprecated-register],[[#include ]],[[int fox=ntohs(1);]])
 
 if test "x$enable_strict_error_checking" != "xno"; then
   SQUID_CFLAGS="$SQUID_CFLAGS $squid_cv_cc_option_werror"
   SQUID_CXXFLAGS="$SQUID_CXXFLAGS $squid_cv_cxx_option_werror"
 fi
 
 if test "x$ac_cv_require_wno_deprecated_register" = "xyes"; then
   SQUID_CXXFLAGS="$SQUID_CXXFLAGS -Wno-deprecated-register"
 fi
 
 # squid_cv_cc_arg_pipe is set by SQUID_CC_GUESS_OPTIONS
 SQUID_CXXFLAGS="$SQUID_CXXFLAGS $squid_cv_cc_arg_pipe"
 SQUID_CFLAGS="$SQUID_CFLAGS $squid_cv_cc_arg_pipe"

=== modified file 'src/clients/FtpRelay.cc'
--- src/clients/FtpRelay.cc	2016-02-23 08:51:22 +
+++ src/clients/FtpRelay.cc	2016-03-11 22:27:55 +
@@ -28,41 +28,41 @@ namespace Ftp
 {
 
 /// An FTP client receiving native FTP commands from our FTP server
 /// (Ftp::Server), forwarding them to the next FTP hop,
 /// and then relaying FTP replies back to our FTP server.
 class Relay: public Ftp::Client
 {
 CBDATA_CLASS(Relay);
 
 public:
 explicit Relay(FwdState *const fwdState);
 virtual ~Relay();
 
 protected:
 const Ftp::MasterState () const;
 Ftp::MasterState ();
 Ftp::ServerState serverState() const { return master().serverState; }
 void serverState(const Ftp::ServerState newState);
 
 /* Ftp::Client API */
-virtual void failed(err_type error = ERR_NONE, int xerrno = 0);
+virtual void failed(err_type error = ERR_NONE, int xerrno = 0, ErrorState *ftperr = nullptr);
 virtual void dataChannelConnected(const CommConnectCbParams );
 
 /* Client API */
 virtual void serverComplete();
 virtual void handleControlReply();
 virtual void processReplyBody();
 virtual void handleRequestBodyProducerAborted();
 virtual bool mayReadVirginReplyBody() const;
 virtual void completeForwarding();
 virtual bool abortOnData(const char *reason);
 
 /* AsyncJob API */
 virtual void start();
 
 void forwardReply();
 void forwardError(err_type error = ERR_NONE, int xerrno = 0);
 void failedErrorMessage(err_type error, int xerrno);
 HttpReply *createHttpReply(const Http::StatusCode httpStatus, const int64_t clen = 0);
 void handleDataRequest();
 void startDataDownload();
@@

[squid-dev] Build failed in Jenkins: trunk-polygraph #956

2016-03-11 Thread noc

See 

--
Started by upstream project "trunk-matrix" build number 573
originally caused by:
 Started by an SCM change
Started by upstream project "trunk-matrix" build number 574
originally caused by:
 Started by an SCM change
Started by upstream project "trunk-matrix" build number 575
originally caused by:
 Started by an SCM change
Started by upstream project "trunk-matrix" build number 576
originally caused by:
 Started by an SCM change
Building remotely on polygraph (12.04 amd64-Ubuntu Ubuntu amd64-Ubuntu-12.04 
Ubuntu-12.04 amd64) in workspace 

Cleaning workspace...
$ bzr checkout --lightweight http://bzr.squid-cache.org/bzr/squid3/trunk/ 

Getting local revision...
$ bzr revision-info -d 
info result: bzr revision-info -d 
 returned 0. Command 
output: "14584 rouss...@measurement-factory.com-20160311180051-n7lxmm28gl17rpup
" stderr: ""
RevisionState revno:14584 
revid:rouss...@measurement-factory.com-20160311180051-n7lxmm28gl17rpup
[trunk-polygraph] $ /bin/sh -xe /tmp/hudson3458800539028691925.sh
+ cd /home/jenkins/squidperf
+ python SquidBasicPerf.py --audited 
http://build.squid-cache.org/job/trunk-polygraph/830/artifact/logs/test.lx 
--jjid 956 --svnurl http://bzr.squid-cache.org/bzr/squid3/trunk/ --jobname 
trunk-polygraph
Test is failed
Build step 'Execute shell' marked build as failure
Archiving artifacts
[description-setter] Could not determine description.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

[squid-dev] Jenkins build is back to normal : trunk-full-matrix » clang,d-fedora-21 #123

2016-03-11 Thread noc

See 


___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

[squid-dev] Build failed in Jenkins: trunk-full-matrix » clang,d-ubuntu-vivid #123

2016-03-11 Thread noc

See 


--
[...truncated 4467 lines...]
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT AclSizeLimit.lo -MD 
-MP -MF .deps/AclSizeLimit.Tpo -c ../../../src/acl/AclSizeLimit.cc  -fPIC -DPIC 
-o .libs/AclSizeLimit.o
depbase=`echo AdaptationService.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT AdaptationService.lo -MD -MP -MF 
$depbase.Tpo -c -o AdaptationService.lo ../../../src/acl/AdaptationService.cc 
&&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationService.lo -MD -MP -MF .deps/AdaptationService.Tpo -c 
../../../src/acl/AdaptationService.cc  -fPIC -DPIC -o .libs/AdaptationService.o
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT AclSizeLimit.lo -MD 
-MP -MF .deps/AclSizeLimit.Tpo -c ../../../src/acl/AclSizeLimit.cc -o 
AclSizeLimit.o >/dev/null 2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT UserData.lo -MD -MP 
-MF .deps/UserData.Tpo -c ../../../src/acl/UserData.cc -o UserData.o >/dev/null 
2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT Gadgets.lo -MD -MP 
-MF .deps/Gadgets.Tpo -c ../../../src/acl/Gadgets.cc -o Gadgets.o >/dev/null 
2>&1
depbase=`echo AdaptationServiceData.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT AdaptationServiceData.lo -MD -MP -MF 
$depbase.Tpo -c -o AdaptationServiceData.lo 
../../../src/acl/AdaptationServiceData.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationServiceData.lo -MD -MP -MF .deps/AdaptationServiceData.Tpo -c 
../../../src/acl/AdaptationServiceData.cc  -fPIC -DPIC -o 
.libs/AdaptationServiceData.o
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationService.lo -MD -MP -MF .deps/AdaptationService.Tpo -c 
../../../src/acl/AdaptationService.cc -o AdaptationService.o >/dev/null 2>&1
depbase=`echo Arp.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT Arp.lo -MD -MP -MF $depbase.Tpo -c -o Arp.lo 
../../../src/acl/Arp.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT Arp.lo -MD -MP -MF 
.deps/Arp.Tpo -c ../../../src/acl/Arp.cc  -fPIC -DPIC -o .libs/Arp.o
depbase=`echo Eui64.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT Eui64.lo -MD -MP -MF $depbase.Tpo -c -o 
Eui64.lo ../../../src/acl/Eui64.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationServiceData.lo -MD -MP -MF

[squid-dev] Build failed in Jenkins: trunk-full-matrix » clang,d-debian-unstable #123

2016-03-11 Thread noc

See 


--
[...truncated 4423 lines...]
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT UserData.lo -MD -MP -MF .deps/UserData.Tpo -c 
../../../../src/acl/UserData.cc -o UserData.o >/dev/null 2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT UrlPort.lo -MD -MP -MF .deps/UrlPort.Tpo -c 
../../../../src/acl/UrlPort.cc -o UrlPort.o >/dev/null 2>&1
depbase=`echo Gadgets.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../../.. -I../../../../include -I../../../../lib 
-I../../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register  -D_REENTRANT -g -O2 -std=c++11 -MT Gadgets.lo -MD -MP 
-MF $depbase.Tpo -c -o Gadgets.lo ../../../../src/acl/Gadgets.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT Gadgets.lo -MD -MP -MF .deps/Gadgets.Tpo -c 
../../../../src/acl/Gadgets.cc  -fPIC -DPIC -o .libs/Gadgets.o
depbase=`echo AclSizeLimit.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../../.. -I../../../../include -I../../../../lib 
-I../../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register  -D_REENTRANT -g -O2 -std=c++11 -MT AclSizeLimit.lo 
-MD -MP -MF $depbase.Tpo -c -o AclSizeLimit.lo 
../../../../src/acl/AclSizeLimit.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT AclSizeLimit.lo -MD -MP -MF .deps/AclSizeLimit.Tpo -c 
../../../../src/acl/AclSizeLimit.cc  -fPIC -DPIC -o .libs/AclSizeLimit.o
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT AclSizeLimit.lo -MD -MP -MF .deps/AclSizeLimit.Tpo -c 
../../../../src/acl/AclSizeLimit.cc -o AclSizeLimit.o >/dev/null 2>&1
depbase=`echo AdaptationService.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../../.. -I../../../../include -I../../../../lib 
-I../../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register  -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationService.lo -MD -MP -MF $depbase.Tpo -c -o AdaptationService.lo 
../../../../src/acl/AdaptationService.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT AdaptationService.lo -MD -MP -MF .deps/AdaptationService.Tpo -c 
../../../../src/acl/AdaptationService.cc  -fPIC -DPIC -o 
.libs/AdaptationService.o
depbase=`echo AdaptationServiceData.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../../.. -I../../../../include -I../../../../lib 
-I../../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register  -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationServiceData.lo -MD -MP -MF $depbase.Tpo -c -o 
AdaptationServiceData.lo ../../../../src/acl/AdaptationServiceData.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../../.. 
-I../../../../include -I../../../../lib -I../../../../src -I../../include 
-Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 
-std=c++11 -MT AdaptationServiceData.lo -MD -MP -MF 
.deps/AdaptationServiceData.Tpo -c ../../../../src/acl/AdaptationServiceData.cc 
 -fPIC -DPIC -o .libs/AdaptationServiceData.o
depbase=`echo Arp.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../../.. -I../../../../include -I../../../../lib 
-I../../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register  -D_REENTRANT -g -O2 -std=c++11 -MT Arp.lo -MD -MP -MF 
$depbase.Tpo -c -o Arp.lo ../../../../src/acl/Arp.cc &&\
mv

[squid-dev] Build failed in Jenkins: trunk-matrix » clang,d-ubuntu-vivid #577

2016-03-11 Thread noc

See 


--
[...truncated 4467 lines...]
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT AclSizeLimit.lo -MD 
-MP -MF .deps/AclSizeLimit.Tpo -c ../../../src/acl/AclSizeLimit.cc  -fPIC -DPIC 
-o .libs/AclSizeLimit.o
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT UserData.lo -MD -MP 
-MF .deps/UserData.Tpo -c ../../../src/acl/UserData.cc -o UserData.o >/dev/null 
2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT AclSizeLimit.lo -MD 
-MP -MF .deps/AclSizeLimit.Tpo -c ../../../src/acl/AclSizeLimit.cc -o 
AclSizeLimit.o >/dev/null 2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT Gadgets.lo -MD -MP 
-MF .deps/Gadgets.Tpo -c ../../../src/acl/Gadgets.cc -o Gadgets.o >/dev/null 
2>&1
depbase=`echo AdaptationService.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT AdaptationService.lo -MD -MP -MF 
$depbase.Tpo -c -o AdaptationService.lo ../../../src/acl/AdaptationService.cc 
&&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationService.lo -MD -MP -MF .deps/AdaptationService.Tpo -c 
../../../src/acl/AdaptationService.cc  -fPIC -DPIC -o .libs/AdaptationService.o
depbase=`echo AdaptationServiceData.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT AdaptationServiceData.lo -MD -MP -MF 
$depbase.Tpo -c -o AdaptationServiceData.lo 
../../../src/acl/AdaptationServiceData.cc &&\
mv -f $depbase.Tpo $depbase.Plo
depbase=`echo Arp.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT Arp.lo -MD -MP -MF $depbase.Tpo -c -o Arp.lo 
../../../src/acl/Arp.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationServiceData.lo -MD -MP -MF .deps/AdaptationServiceData.Tpo -c 
../../../src/acl/AdaptationServiceData.cc  -fPIC -DPIC -o 
.libs/AdaptationServiceData.o
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT Arp.lo -MD -MP -MF 
.deps/Arp.Tpo -c ../../../src/acl/Arp.cc  -fPIC -DPIC -o .libs/Arp.o
depbase=`echo Eui64.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/bash ../../libtool  --tag=CXX   --mode=compile ccache clang++ 
-DHAVE_CONFIG_H   -I../../.. -I../../../include -I../../../lib -I../../../src 
-I../../include -Werror -Qunused-arguments -Wno-deprecated-register  
-D_REENTRANT -g -O2 -std=c++11 -MT Eui64.lo -MD -MP -MF $depbase.Tpo -c -o 
Eui64.lo ../../../src/acl/Eui64.cc &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT 
AdaptationService.lo -MD -MP -MF .deps/AdaptationService.Tpo -c 
../../../src/acl/AdaptationService.cc -o AdaptationService.o >/dev/null 2>&1
libtool: compile:  ccache clang++ -DHAVE_CONFIG_H -I../../.. -I../../../include 
-I../../../lib -I../../../src -I../../include -Werror -Qunused-arguments 
-Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -MT Eui64.lo -MD -MP 
-MF .deps/Eui64.Tpo -c ../../../src/acl/Eui64.cc

Re: [squid-dev] [PATCH] Bug 7: Headers are not updated on disk after 304s

2016-03-11 Thread Alex Rousskov

On 03/11/2016 02:17 AM, Amos Jeffries wrote:
> On 11/03/2016 2:59 p.m., Alex Rousskov wrote:
>> The attached compressed patch fixes a 15+ years old Bug #7 [1] for
>> the shared memory cache and rock cache_dirs. I am not aware of anybody
>> working on ufs-based cache_dirs, but this patch provides a Store API and
>> a cache_dir example on how to fix those as well.
>>
>>   [1] http://bugs.squid-cache.org/show_bug.cgi?id=7

> Ah. I'm getting deja-vu on this. Thought those two cache types were
> fixed long ago and recent talk was you were working on the UFS side of it.

There was some noise about this bug and related issues some months ago.
It was easy to get confused by all the mis[leading]information being
posted on bugzilla, including reports that "the bug is fixed" for some
ufs-based cache_dirs. I tried to correct those reports but failed to
convince people that they do not see what they think they see.

After this patch, the following cache stores (and only them) should
support header updates:

  * non-shared memory cache (in non-SMP Squids only)
  * shared memory cache
  * rock cache_dir

Needless to say, the posted patch does not fix all the problems with
header updates, even for the above stores. For example, the code that
decides which headers to update may still violate HTTP in some ways (I
have not checked). The patch "just" writes the headers computed by Squid
to shared memory cache and to rock cache_dirs.

Moreover, given the [necessary] complexity of the efficient update code
combined with the [unnecessary] complexity of some old Store APIs, I
would be surprised if there are no new bugs or problems introduced by
our changes. I am not aware of any, but we continue to test and plan to
fix the ones we find.

>> Besides unavoidable increase in rock-based caching code complexity, the
>> [known] costs of this fix are:
>>
>> 1. 8 additional bytes per cache entry for shared memory cache and rock
>> cache_dirs. Much bigger but short-lived RAM _savings_ for rock
>> cache_dirs (due to less RAM-hungry index rebuild code) somewhat mitigate
>> this RAM usage increase.
>>
>> 2. Increased slot fragmentation when updated headers are slightly larger
>> than old ones. This can probably be optimized away later if needed by
>> padding HTTP headers or StoreEntry metadata.
>>
>> 3. Somewhat slower rock cache_dir index rebuild time. IMO, this should
>> eventually be dealt with by not rebuilding the index on most startups at
>> all (rather than focusing on the index rebuild optimization).
> 
> Hmm. Nod, agreed on the long-term approach.
> 
>>
>> The patch preamble (also quoted below) contains more technical details,
>> including a list of side changes that, ideally, should go in as separate
>> commits. The posted patch is based on our bug7 branch on lp[2] which has
>> many intermediate commits. I am not yet sure whether it makes sense to
>> _merge_ that branch into trunk or simply commit it as a single/atomic
>> change (except for those side changes). Opinions welcomed.

> Do you know how to do a merge like that with bzr properly?
>  My experience has been that it only likes atomic-like merges.

I sense a terminology conflict. By "merge", I meant "bzr merge". Trunk
already has many merged branches, of course:

  revno: 14574 [merge]
  revno: 14573 [merge]
  revno: 14564 [merge]
  ...

By single/atomic change, I meant "patch < bug7.patch". Merges preserve
individual branch commits which is good when those commits are valuable
and bad when those commits are noise. In case of our bug7 branch, it is
a mixture of valuable stuff and noise. I decided to do a single/atomic
change to avoid increasing the noise level.

> in src/StoreIOState.h:
> 
> * if the XXX about file_callback can the removal TODO be enacted ?
>  - at least as one of the side-change patches

Yes, of course, but out of this project scope. We already did the
difficult part -- detected and verified that the API is unused.
Hopefully, somebody will volunteer to do the rest (and to take the
responsibility for it).

> * the docs on touchingStoreEntry() seem to contradict your description
> of how the entry chains work. Now. You said readers could read whatever
> chain they were attached to after the update switch. The doc says they
> only ever read the primary.

Done: Clarified that the primary chain (which the readers always start
with) may become secondary later:

> // Tests whether we are working with the primary/public StoreEntry chain.
> // Reads start reading the primary chain, but it may become secondary.
> // There are two store write kinds:
> // * regular writes that change (usually append) the entry visible to all 
> and
> // * header updates that create a fresh chain (while keeping the stale 
> one usable).
> bool touchingStoreEntry() const;

The readers do not matter in the current code because reading code does
not use this method, but that may change in the future, of course.

> in src/fs/rock/RockHeaderUpdater.cc:
> 
>

Re: [squid-dev] [PATCH] Ensure any previous value in lastAclData is free'd

2016-03-11 Thread Amos Jeffries

On 11/03/2016 5:33 p.m., Nathan Hoad wrote:
> Sure, that seems straightforward enough. Attached is a patch that
> migrates lastAclData to an SBuf.
> 

Thank you. I have added an isEmpty() check before calling c_str(), and
applied as trunk rev.14579.

Amos

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] [PATCH] Bug 7: Headers are not updated on disk after 304s

2016-03-11 Thread Amos Jeffries

On 11/03/2016 2:59 p.m., Alex Rousskov wrote:
> Hello,
> 
> The attached compressed patch fixes a 15+ years old Bug #7 [1] for
> the shared memory cache and rock cache_dirs. I am not aware of anybody
> working on ufs-based cache_dirs, but this patch provides a Store API and
> a cache_dir example on how to fix those as well.
> 
>   [1] http://bugs.squid-cache.org/show_bug.cgi?id=7
> 

Ah. I'm getting deja-vu on this. Thought those two cache types were
fixed long ago and recent talk was you were working on the UFS side of it.

Sigh. Oh well.


> Besides unavoidable increase in rock-based caching code complexity, the
> [known] costs of this fix are:
> 
> 1. 8 additional bytes per cache entry for shared memory cache and rock
> cache_dirs. Much bigger but short-lived RAM _savings_ for rock
> cache_dirs (due to less RAM-hungry index rebuild code) somewhat mitigate
> this RAM usage increase.
> 
> 2. Increased slot fragmentation when updated headers are slightly larger
> than old ones. This can probably be optimized away later if needed by
> padding HTTP headers or StoreEntry metadata.
> 
> 3. Somewhat slower rock cache_dir index rebuild time. IMO, this should
> eventually be dealt with by not rebuilding the index on most startups at
> all (rather than focusing on the index rebuild optimization).

Hmm. Nod, agreed on the long-term approach.

> 
> The patch preamble (also quoted below) contains more technical details,
> including a list of side changes that, ideally, should go in as separate
> commits. The posted patch is based on our bug7 branch on lp[2] which has
> many intermediate commits. I am not yet sure whether it makes sense to
> _merge_ that branch into trunk or simply commit it as a single/atomic
> change (except for those side changes). Opinions welcomed.

Do you know how to do a merge like that with bzr properly?
 My experience has been that it only likes atomic-like merges.

> 
>   [2] https://code.launchpad.net/~measurement-factory/squid/bug7
> 
> 
> ---
> Bug 7: Update cached entries on 304 responses
> 
> New Store API to update entry metadata and headers on 304s.
> Support entry updates in shared memory cache and rock cache_dirs.
> 
> * Highlights:
> 
> 1. Atomic StoreEntry metadata updating
> 
>StoreEntry metadata (swap_file_sz, timestamps, etc.) is used
>throughout Squid code. Metadata cannot be updated atomically because
>it has many fields, but a partial update to those fields causes
>assertions. Still, we must update metadata when updating HTTP
>headers. Locking the entire entry for a rewrite does not work well
>because concurrent requests will attempt to download a new entry
>copy, defeating the very HTTP 304 optimization we want to support.
> 
>Ipc::StoreMap index now uses an extra level of indirection (the
>StoreMap::fileNos index) which allows StoreMap control which
>anchor/fileno is associated with a given StoreEntry key. The entry
>updating code creates a disassociated (i.e., entry/key-less) anchor,
>writes new metadata and headers using that new anchor, and then
>_atomically_ switches the map to use that new anchor. This allows old
>readers to continue reading using the stale anchor/fileno as if
>nothing happened while a new reader gets the new anchor/fileno.

:-)

> 
>Shared memory usage increase: 8 additional bytes per cache entry: 4
>for the extra level of indirection (StoreMapFileNos) plus 4 for
>splicing fresh chain prefix with the stale chain suffix
>(StoreMapAnchor::splicingPoint). However, if the updated headers are
>larger than the stale ones, Squid will allocate shared memory pages
>to accommodate for the increase, leading to shared memory
>fragmentation/waste for small increases.

> 
> 2. Revamped rock index rebuild process
> 
>The index rebuild process had to be completely revamped because
>splicing fresh and stale entry slot chain segments implies tolerating
>multiple entry versions in a single chain and the old code was based
>on the assumption that different slot versions are incompatible. We
>were also uncomfortable with the old cavalier approach to accessing
>two differently indexed layers of information (entry vs. slot) using
>the same set of class fields, making it trivial to accidentally
>access entry data while using slot index.
> 
>During the rewrite of the index rebuilding code, we also discovered a
>way to significantly reduce RAM usage for the index build map (a
>temporary object that is allocated in the beginning and freed at the
>end of the index build process). The savings depend on the cache
>size: A small cache saves about 30% (17 vs 24 bytes per entry/slot)
>while a 1TB cache_dir with 32KB slots (which implies uneven
>entry/slot indexes) saves more than 50% (~370MB vs. ~800MB).
> 
>Adjusted how invalid slots are counted. The code was sometimes
>counting invalid entries and sometimes invalid

[squid-dev] [PATCH] g++ -Woverloaded-virtual

[squid-dev] Build failed in Jenkins: trunk-polygraph #956

[squid-dev] Jenkins build is back to normal : trunk-full-matrix » clang,d-fedora-21 #123

[squid-dev] Build failed in Jenkins: trunk-full-matrix » clang,d-ubuntu-vivid #123

[squid-dev] Build failed in Jenkins: trunk-full-matrix » clang,d-debian-unstable #123

[squid-dev] Build failed in Jenkins: trunk-matrix » clang,d-ubuntu-vivid #577

Re: [squid-dev] [PATCH] Bug 7: Headers are not updated on disk after 304s

Re: [squid-dev] [PATCH] Ensure any previous value in lastAclData is free'd

Re: [squid-dev] [PATCH] Bug 7: Headers are not updated on disk after 304s

9 matches

Site Navigation

Mail list logo

Footer information