From 10ea85ee45f4dd2ce818889616240ca3b1fd3c3e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Oct 2018 21:02:22 +0200 Subject: [PATCH] tools/frr: get rid of the bash array and "bB" With a little shell-fu, this can actually be escaped properly. Signed-off-by: David Lamparter --- redhat/daemons | 2 +- redhat/frr.init | 42 ++++++++++++++------------ tools/etc/frr/daemons.conf | 3 +- tools/frr.in | 60 ++++++++++++++++++++------------------ 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/redhat/daemons b/redhat/daemons index c301a1c23a..7f3ff36df9 100644 --- a/redhat/daemons +++ b/redhat/daemons @@ -35,7 +35,7 @@ # group "frrvty" and set to ug=rw,o= though. Check /etc/pam.d/frr, too. # watchfrr_enable=yes -watchfrr_options=("-b_" "-r/usr/lib/frr/frr_restart_%s" "-s/usr/lib/frr/frr_start_%s" "-k/usr/lib/frr/frr_stop_%s") +watchfrr_options="-r '/usr/lib/frr/frr restart %s' -s '/usr/lib/frr/frr start %s' -k '/usr/lib/frr/frr stop %s'" # zebra=no bgpd=no diff --git a/redhat/frr.init b/redhat/frr.init index 47a92eed32..b59656adcd 100755 --- a/redhat/frr.init +++ b/redhat/frr.init @@ -107,22 +107,28 @@ check_daemon() # The Frr daemons creates the pidfile when starting. start() { + local dmn inst + dmn="$1" + inst="$2" + ulimit -n $MAX_FDS > /dev/null 2> /dev/null - if [ "$1" = "watchfrr" ]; then + if [ "$dmn" = "watchfrr" ]; then # We may need to restart watchfrr if new daemons are added and/or # removed - if started "$1" ; then + if started "$dmn" ; then stop watchfrr else # Echo only once. watchfrr is printed in the stop above - echo -n " $1" + echo -n " $dmn" fi if [ -e /var/run/frr/watchfrr.started ] ; then rm /var/run/frr/watchfrr.started fi - daemon --pidfile=`pidfile $1` "$D_PATH/$1" -d "${watchfrr_options[@]}" + # redhat /etc/init.d/functions daemon() re-expands args :( + # eval "set - $watchfrr_options" + daemon --pidfile=`pidfile $dmn` "$D_PATH/$dmn" -d "$watchfrr_options" RETVAL=$? [ $RETVAL -ne 0 ] && break for i in `seq 1 10`; @@ -135,25 +141,25 @@ start() fi done RETVAL=1 - elif [ -n "$2" ]; then - echo -n " $1-$2" - if ! check_daemon $1 $2 ; then + elif [ -n "$inst" ]; then + echo -n " $dmn-$inst" + if ! check_daemon $dmn $inst ; then echo -n " (binary does not exist)" return; fi - daemon --pidfile=`pidfile $1-$2` "$D_PATH/$1" -d `eval echo "$""$1""_options"` -n "$2" + daemon --pidfile=`pidfile $dmn-$inst` "$D_PATH/$dmn" -d `eval echo "$""$dmn""_options"` -n "$inst" RETVAL=$? else - echo -n " $1 " - if ! check_daemon $1; then + echo -n " $dmn " + if ! check_daemon $dmn; then echo " (binary does not exist)" return; fi - daemon --pidfile=`pidfile $1` "$D_PATH/$1" -d `eval echo "$""$1""_options"` + daemon --pidfile=`pidfile $dmn` "$D_PATH/$dmn" -d `eval echo "$""$dmn""_options"` RETVAL=$? fi echo - [ $RETVAL -eq 0 ] && touch /var/lock/subsys/$1 + [ $RETVAL -eq 0 ] && touch /var/lock/subsys/$dmn return $RETVAL } @@ -223,11 +229,9 @@ start_watchfrr() fi # Check variable type - if ! declare -p watchfrr_options | grep -q '^declare \-a'; then - echo - echo "ERROR: The variable watchfrr_options from /etc/frr/daemons must be a BASH array!" - echo "ERROR: Please convert config file and restart!" - exit 1 + if declare -p watchfrr_options | grep -q '^declare \-a'; then + # old array support + watchfrr_options="${watchfrr_options[@]}" fi # Which daemons have been started? @@ -241,13 +245,13 @@ start_watchfrr() eval "inst_disable=\${${daemon_name}_${inst}}" if [ -z ${inst_disable} ] || [ ${inst_disable} != 0 ]; then if check_daemon $daemon_name $inst; then - watchfrr_options+=("${daemon_name}-${inst}") + watchfrr_options="$watchfrr_options ${daemon_name}-${inst}" fi fi done else if check_daemon $daemon_name; then - watchfrr_options+=($daemon_name) + watchfrr_options="$watchfrr_options $daemon_name" fi fi found_one=1 diff --git a/tools/etc/frr/daemons.conf b/tools/etc/frr/daemons.conf index 94221301eb..1f608cfe08 100644 --- a/tools/etc/frr/daemons.conf +++ b/tools/etc/frr/daemons.conf @@ -24,8 +24,7 @@ fabricd_options=" --daemon -A 127.0.0.1" # The list of daemons to watch is automatically generated by the init script. watchfrr_enable=yes - -watchfrr_options=(-d -r /usr/lib/frr/frrbBrestartbB%s -s /usr/lib/frr/frrbBstartbB%s -k /usr/lib/frr/frrbBstopbB%s -b bB) +watchfrr_options="-d -r '/usr/lib/frr/frr restart %s' -s '/usr/lib/frr/frr start %s' -k '/usr/lib/frr/frr stop %s'" # If valgrind_enable is 'yes' the frr daemons will be started via valgrind. # The use case for doing so is tracking down memory leaks, etc in frr. diff --git a/tools/frr.in b/tools/frr.in index ec383bc5a0..a443191fd0 100755 --- a/tools/frr.in +++ b/tools/frr.in @@ -115,41 +115,45 @@ check_daemon() # The Frr daemons creates the pidfile when starting. start() { - ulimit -n $MAX_FDS - if [ "$1" = "watchfrr" ]; then + local dmn inst + dmn="$1" + inst="$2" + + ulimit -n $MAX_FDS > /dev/null 2> /dev/null + if [ "$dmn" = "watchfrr" ]; then # We may need to restart watchfrr if new daemons are added and/or # removed - if started "$1" ; then + if started "$dmn" ; then stop watchfrr else # Echo only once. watchfrr is printed in the stop above - echo -n " $1" + echo -n " $dmn" fi - + eval "set - $watchfrr_options" ${SSD} \ --start \ - --pidfile=`pidfile $1` \ - --exec "$D_PATH/$1" \ + --pidfile=`pidfile $dmn` \ + --exec "$D_PATH/$dmn" \ -- \ - "${watchfrr_options[@]}" + "$@" - elif [ -n "$2" ]; then - echo -n " $1-$2" - if ! check_daemon $1 $2 ; then + elif [ -n "$inst" ]; then + echo -n " $dmn-$inst" + if ! check_daemon $dmn $inst ; then echo -n " (binary does not exist)" return; fi ${SSD} \ --start \ - --pidfile=`pidfile $1-$2` \ - --exec "$D_PATH/$1" \ + --pidfile=`pidfile $dmn-$inst` \ + --exec "$D_PATH/$dmn" \ -- \ - `eval echo "$""$1""_options"` -n "$2" + `eval echo "$""$dmn""_options"` -n "$inst" else - if ! check_daemon $1; then + if ! check_daemon $dmn; then echo -n " (binary does not exist)" return; fi @@ -157,22 +161,22 @@ start() if [ "$valgrind_enable" = "yes" ]; then ${SSD} \ --start \ - --pidfile=`pidfile $1` \ + --pidfile=`pidfile $dmn` \ --exec "$valgrind" \ - -- --trace-children=no --leak-check=full --log-file=/var/log/frr/$1-valgrind.log $D_PATH/$1 \ - `eval echo "$""$1""_options"` + -- --trace-children=no --leak-check=full --log-file=/var/log/frr/$dmn-valgrind.log $D_PATH/$dmn \ + `eval echo "$""$dmn""_options"` else ${SSD} \ --start \ - --pidfile=`pidfile $1` \ - --exec "$D_PATH/$1" \ + --pidfile=`pidfile $dmn` \ + --exec "$D_PATH/$dmn" \ -- \ - `eval echo "$""$1""_options"` + `eval echo "$""$dmn""_options"` fi fi # Start the staticd automatically - if [ "$1" = "zebra" ]; then + if [ "$dmn" = "zebra" ]; then echo -n "starting staticd since zebra is running" if ! check_daemon staticd ; then echo -n " (binary does not exist)" @@ -269,11 +273,9 @@ start_watchfrr() fi # Check variable type - if ! declare -p watchfrr_options | grep -q '^declare \-a'; then - echo - echo "ERROR: The variable watchfrr_options from /etc/frr/debian.cnf must be a BASH array!" - echo "ERROR: Please convert config file and restart!" - exit 1 + if declare -p watchfrr_options | grep -q '^declare \-a'; then + # old array support + watchfrr_options="${watchfrr_options[@]}" fi # Which daemons have been started? @@ -287,13 +289,13 @@ start_watchfrr() eval "inst_disable=\${${daemon_name}_${inst}}" if [ -z ${inst_disable} ] || [ ${inst_disable} != 0 ]; then if check_daemon $daemon_name $inst; then - watchfrr_options+=("${daemon_name}-${inst}") + watchfrr_options="$watchfrr_options ${daemon_name}-${inst}" fi fi done else if check_daemon $daemon_name; then - watchfrr_options+=($daemon_name) + watchfrr_options="$watchfrr_options $daemon_name" fi fi found_one=1 -- 2.39.5