Re: [PATCH] squid: Update-suggestion for initscript

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