Issue in a Bash loop

Question

just a quick question with something I'm not understanding. Everything is working fine - I'm just trying to get my head around something.

I have a very simple Bash script that is restarting dnsmasq once per day:

LogFile="/var/log/logfile"
declare -a CMD_AR=()
declare -a ECHO_OUT=(stopping starting)
STOP=$(service dnsmasq \stop)
SLEEP=$(\sleep 15)
START=$(service dnsmasq \start)
CMD_AR+=($STOP)
CMD_AR+=($START)

z=0
while [[ $z -le 1 ]]; do
      DT=`date +%c`
      echo "$DT - ${ECHO_OUT[$z]} dnsmasq..." >> ${LogFile}
      eval ${CMD_AR[$z]} >> ${LogFile} 2>&1
      unset DT
      eval ${SLEEP}
      z=$[$z+1]
done

So...this works, however, the DT variable never changes. So my log file reads:

Tue 31 Oct 2017 10:57:07 PM MDT - stopping dnmasq
Tue 31 Oct 2017 10:57:07 PM MDT - starting dnmasq

Shouldn't the time string be at least 15 (the value of sleep 15) seconds different? I'm failing to understand why the loop is not re-computing the DT variable - anyone?

Also, I'll take any suggestions on my code as well - I'm a total hack when it comes to scripting.

while [[ $z -l e 1 ]]; do <-- is this intentional you could avoid while if this is intentional , BTW SLEEP=$(\sleep 15) => SLEEP="sleep 15" — asio_guy
– asio_guy, Commented Nov 1, 2017 at 5:28

codeforester · Accepted Answer · 2017-11-01 19:59:22Z

3

The issue is in this line:

SLEEP=$(\sleep 15)

Which sets SLEEP variable to a null string since the command sleep 15 generates no output.

Change it to:

SLEEP="sleep 15"

to solve your problem.

The line

START=$(service dnsmasq \start)

has the same problem.

Your script seems overly complicated - get it checked through shellcheck. In general, it is not a good idea to store commands in a variable and execute them through eval - see BashFAQ/050. Your code can be rewritten this way:

LogFile="/var/log/logfile"
z=0
while ((z <= 1)); do
    if ((z == 0)); then
      echo "$(date) - stopping dnsmasq..." >> "${LogFile}"
      service dnsmasq stop >> "${LogFile}" 2>&1
    else
      echo "$(date) - starting dnsmasq..." >> "${LogFile}"
      service dnsmasq start >> "${LogFile}" 2>&1
    fi
    sleep 15
    ((z++))
done

Improvements done:

Use (( .. )) for arithmetic operations and checks
Wrap variables in double quotes to prevent word splitting
Remove indirect execution with eval
Eliminate the array - we don't need it here
Make the code more readable

The loop seems unnecessary here. The code could probably be written even more concisely as:

LogFile="/var/log/logfile"
exec >"$LogFile" 2>&1
echo "$(date) - stopping dnsmasq..."
service dnsmasq stop
sleep 15
echo "$(date) - starting dnsmasq..."
service dnsmasq start

edited Nov 1, 2017 at 19:59

answered Nov 1, 2017 at 5:24

codeforester

43.8k21 gold badges122 silver badges159 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Josh Williamson Over a year ago

OK, thanks - Yeah I realize that it's a little bit complicated, I just did the array just to do it really - it's also not all there is to my script, so I already had the array defined - but it's definitely overkill. I was also under the impression that it was advisable to use eval - in fact, I originally had "service dnsmasq restart" and while it was working, I would occasionally get an error message in the log about "bad command". Anyway, thanks for the help - like I said, I'm a newb -Josh

Josh Williamson Over a year ago

Oh, and thank you for the exec command - that's awesome and shellcheck :)

codeforester Over a year ago

Glad it helped. What to do when someone answers my question.

Collectives™ on Stack Overflow

Issue in a Bash loop

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related