squid: Update-suggestion for initscript
Message ID | 1462992406-1395-1-git-send-email-matthias.fischer@ipfire.org |
---|---|
State | Superseded |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (hedwig.ipfire.org [172.28.1.200]) by web02.ipfire.org (Postfix) with ESMTP id 38BF562476 for <patchwork@ipfire.org>; Wed, 11 May 2016 20:46:55 +0200 (CEST) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 6179EA4D; Wed, 11 May 2016 20:46:54 +0200 (CEST) Received: from Devel.localdomain (p5DD837AD.dip0.t-ipconnect.de [93.216.55.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id CA192A49 for <development@lists.ipfire.org>; Wed, 11 May 2016 20:46:50 +0200 (CEST) From: Matthias Fischer <matthias.fischer@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH] squid: Update-suggestion for initscript Date: Wed, 11 May 2016 20:46:46 +0200 Message-Id: <1462992406-1395-1-git-send-email-matthias.fischer@ipfire.org> X-Mailer: git-send-email 2.8.2 X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Message
Matthias Fischer
May 12, 2016, 4:46 a.m. UTC
Path fix for squid.
Changed loop detecting if 'squid' is still running.
Added '/bin/rm -f /var/log/cache/swap.state' after killing 'squid'.
Changed flush option to '/bin/rm -f /var/log/cache/swap.state'.
Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
---
src/initscripts/init.d/squid | 47 +++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 18 deletions(-)
Comments
Hi, great work so far getting to the bottom of all this! Just a few comments and questions: On Wed, 2016-05-11 at 20:46 +0200, Matthias Fischer wrote: > Path fix for squid. > > Changed loop detecting if 'squid' is still running. > > Added '/bin/rm -f /var/log/cache/swap.state' after killing 'squid'. Please include why you added this. I can see that you added the command but I am not sure why this is needed. You should include this as a comment in the script. > > Changed flush option to '/bin/rm -f /var/log/cache/swap.state'. > > Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org> > --- > src/initscripts/init.d/squid | 47 +++++++++++++++++++++++++++-------------- > --- > 1 file changed, 29 insertions(+), 18 deletions(-) > > diff --git a/src/initscripts/init.d/squid b/src/initscripts/init.d/squid > index c641c7d..eecad12 100644 > --- a/src/initscripts/init.d/squid > +++ b/src/initscripts/init.d/squid > @@ -33,21 +33,22 @@ transparent() { > CONN_TYPE=`echo "$LINE" | awk -F, '{ print $5 }'` > if [ "$CONN_TYPE" != "net" ]; then > continue > - fi > - iptables -t nat -A SQUID -i $1 -p tcp -d `echo "$LINE" | > awk -F, '{ print $13 }'` --dport 80 -j RETURN > + fi > + iptables -t nat -A SQUID -i $1 -p tcp -d `echo "$LINE" | awk > -F, '{ print $13 }'` --dport 80 -j RETURN > done < $FILE > - > + > if [ "$RED_TYPE" == "STATIC" ]; then > iptables -t nat -A SQUID -i $1 -p tcp -d > $RED_NETADDRESS/$RED_NETMASK --dport 80 -j RETURN > fi > - > + > iptables -t nat -A SQUID -i $1 -p tcp -d $LOCALIP --dport 80 > -j RETURN > - > + > iptables -t nat -A SQUID -i $1 -p tcp --dport 80 -j REDIRECT > --to-port "${TRANSPARENT_PORT}" > } Not sure what happened here. The lines do not seem to have changed. If your editor changed that, please reconfigure it to not do this any more. If this was intentional, please send a second patch for any changes that have no relevance to what is fixed in this commit. > case "$1" in > start) > + > getpids "squid" > > if [ -n "${pidlist}" ]; then Same here. When ever re-formatting code is needed, do that in a different patch. These empty lines there should not be there though. > @@ -93,14 +94,16 @@ case "$1" in > ;; > > stop) > + > iptables -t nat -F SQUID > if [ -e /var/run/squid.pid ]; then > boot_mesg "Stopping Squid Proxy Server..." > - squid -k shutdown >/dev/null 2>&1 > + /usr/sbin/squid -k shutdown >/dev/null 2>&1 > evaluate_retval Why is the full path name needed? It shouldn't. > > # Stop squidGuard, updxlrator, squidclamav > - # and redirect_wrappers. > + # and redirect_wrappers. > + > killproc /usr/bin/squidGuard >/dev/null & > killproc /usr/sbin/updxlrator >/dev/null & > killproc /usr/bin/squidclamav >/dev/null & > @@ -109,22 +112,30 @@ case "$1" in > # Wait until all redirectors have been stopped. > wait > > - # If squid is still running, wait up to 30 seconds > + # If squid is still running, wait up to 120 seconds > # before we go on to kill it. > - counter=30 > > - while [ ${counter} -gt 0 ]; do > - statusproc /usr/sbin/squid >/dev/null && > break; > + n=0 > + while /usr/sbin/squid -k check && [ $n -lt 120 ]; do > sleep 1 > - counter=$(( ${counter} - 1)) > + echo -n . > + n=`/usr/bin/expr $n + 1` > + > done You can use the same syntax that was used previously to increment n: n=$(( ${n} + 1 )) That avoids starting a new process and compute the result and lets the shell do it. > > - # Kill squid service, if still running. > +# Kill squid service, if still running. > killproc /usr/sbin/squid >/dev/null Comments should be indented as well. > > - # Trash remain pid file from squid. > - rm -rf /var/run/squid.pid > +# Delete - damaged - /var/log/cache/swap.state > + /bin/rm -f /var/log/cache/swap.state > + > + fi See comment above. > + > + # Delete remaining pid file from squid if it STILL exists. > + if [ -e '/var/run/squid.pid' ]; then > + rm -rf /var/run/squid.pid > fi If you run rm with the -f switch, it will delete the file if it is there and do nothing if it does not exit. So checking before is not needed. > + > ;; > > restart) > @@ -141,14 +152,14 @@ case "$1" in > statusproc /usr/sbin/squid > statusproc /usr/lib/squid/unlinkd > ;; > - > + > flush) > $0 stop > - echo > /var/log/cache/swap.state > - chown squid.squid /var/log/cache/swap.state > + /bin/rm -f /var/log/cache/swap.state > sleep 1 > $0 start > ;; > + > setperms) > chown -R nobody.squid /var/updatecache/ > ;; Best, -Michael
On 12.05.2016 00:01, Michael Tremer wrote: > Hi, Ahmt, ;-) > great work so far getting to the bottom of all this! > > Just a few comments and questions: > ... I sorted it out and - not knowing to do it any other way - wrote two completely new commits - please see below. Since I changed a lot last night since this one, I thought this would be better. Best, Matthias