Convert repeated code into a loop

Question

I have a router running DD-WRT that has a start up script used to wake my computer on LAN requests on certain ports. I found this shell script here and changed it to allow for wake on multiple ports. I tried doing a for loop so I wouldn't have to repeat any code, but that didn't work. That only thing that worked was:

#!/bin/sh
#Enable JFFS2 and place script in /jffs/ then run on startup in web interface.
#You can check the log from http://192.168.1.1/user/wol.html
INTERVAL=1
PINGTIME=1
OLD=""
PORTPLEX=32400
PORTFTPCTRL=20
PORTFTPDATA=21
PORTRDP=3389
WOLPORT=9
TARGET=192.168.1.8
BROADCAST=192.168.1.255
MAC=aa:bb:cc:dd:ee:ff
WOL=/usr/sbin/wol
LOGFILE="/tmp/www/wol.html"
echo "<meta http-equiv=\"refresh\" content=\"10\">" > $LOGFILE
echo "["`date`"] AUTO WOL Script started. <br>" >> $LOGFILE

while sleep $INTERVAL;do

NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTPLEX"'/ {print }' | tail -1`
SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTPLEX"'/ {print $7}' | tail -1`
LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTPLEX"'/'`
if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
   if ping -qw $PINGTIME $TARGET >/dev/null; then
      echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
   else
      echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
      $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
      echo "<br>" >> $LOGFILE
      sleep 1
   fi
   OLD=$NEW
fi

NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPCTRL"'/ {print }' | tail -1`
SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPCTRL"'/ {print $7}' | tail -1`
LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPCTRL"'/'`
if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
   if ping -qw $PINGTIME $TARGET >/dev/null; then
      echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
   else
      echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
      $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
      echo "<br>" >> $LOGFILE
      sleep 1
   fi
   OLD=$NEW
fi

NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPDATA"'/ {print }' | tail -1`
SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPDATA"'/ {print $7}' | tail -1`
LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTFTPDATA"'/'`
if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
   if ping -qw $PINGTIME $TARGET >/dev/null; then
      echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
   else
      echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
      $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
      echo "<br>" >> $LOGFILE
      sleep 1
   fi
   OLD=$NEW
fi

NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTRDP"'/ {print }' | tail -1`
SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTRDP"'/ {print $7}' | tail -1`
LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORTRDP"'/'`
if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
   if ping -qw $PINGTIME $TARGET >/dev/null; then
      echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
   else
      echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
      $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
      echo "<br>" >> $LOGFILE
      sleep 1
   fi
   OLD=$NEW
fi

done

I'm guessing either (1) I don't really understand how for-loops work in bash, or (2) DD-WRT's shell scripting is different than other versions of Linux, or both.

How would I convert this portion of code to be executed in a for-loop that loops over each port?

NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/ {print }' | tail -1`
SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/ {print $7}' | tail -1`
LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/'`
if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
   if ping -qw $PINGTIME $TARGET >/dev/null; then
      echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
   else
      echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
      $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
      echo "<br>" >> $LOGFILE
      sleep 1
   fi
   OLD=$NEW
fi

Thanks!

Could you state what changes there are between each block of repeated code? Factor those out as variables and put that code into a function, passing the variables as parameters. That would reduce the code to a series of function calls. Then introduce the loop to repeat the function call. — cdarke, Jul 20 '18 at 18:57
@cdarke Thank you, I removed the `bash` tag. Well, the only difference is in the first three lines, which variable I'm using for $PORT. — MattR, Jul 20 '18 at 20:51
@cdarke, am I correct in assuming that a function in a loop would be the same as Andre Gelinas's answer? — MattR, Jul 20 '18 at 21:02
No. The function would be declared outside the loop. The function *call* would be inside the loop - just one line. But I'm not saying @AndreGelinas is wrong, its just a different style and approach. — cdarke, Jul 20 '18 at 21:19

Andre Gelinas · Answer 1 · 2018-07-20T21:07:19.047

0

Have you tried ? :

for PORT in $PORTPLEX $PORTFTPCTRL $PORTFTPDATA $PORTRDP; do
            NEW=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/ {print }' | tail -1`
    SRC=`dmesg | awk -F'[=| ]' '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/ {print $7}' | tail -1`
    LINE=`dmesg | awk '/ACCEPT/ && /DST='"$TARGET"'/ && /DPT='"$PORT"'/'`
    if [ "$NEW" != "" -a "$NEW" != "$OLD" ]; then
       if ping -qw $PINGTIME $TARGET >/dev/null; then
          echo "NOWAKE $TARGET was accessed by $SRC and is already alive at" `date` "<br>">> $LOGFILE
       else
          echo "WAKE $SRC causes wake on lan at" `date` "<br>">> $LOGFILE
          $WOL -i $BROADCAST -p $WOLPORT $MAC >> $LOGFILE
          echo "<br>" >> $LOGFILE
          sleep 1
       fi
       OLD=$NEW
    fi
done

edited Jul 20 '18 at 21:07

answered Jul 20 '18 at 19:05

Andre Gelinas

912
1
8
10

Yeah, that's what I tried originally, but it didn't work... It seems like it should, but I wonder if DD-WRT just doesn't like for-loops?? Doubtful, but who knows. – MattR Jul 20 '18 at 20:52
**Correction**: originally, I put `for PORT in $PORTPLEX $PORTFTPCTRL $PORTFTPDATA $PORTRDP` and then `do` on the next line. So the differences in my code are that `PORT` is in all caps and `do` is on a newline. Does this make a difference? – MattR Jul 20 '18 at 20:54
No...my mistake, sorry..."port" should be "PORT" since you're using $PORT. I've edited accordingly. And no, shouldn't make much of a difference for the "do", that's my way of writing that's all. – Andre Gelinas Jul 20 '18 at 21:09
Ah okay, thanks for clarifying. I wonder why it doesn't work then? Maybe I'll look at some more DD-WRT-centric posts and see if I find anything. – MattR Jul 20 '18 at 21:15
What you could do is inserting some echo before the if/else/fi to see what's really in the 3 variables. – Andre Gelinas Jul 20 '18 at 21:18
I found something [here](https://wiki.dd-wrt.com/wiki/index.php/Useful_Scripts#Directory_Listing_for_DD-WRT_Micro) that gives an example of for-loop in a DD-WRT shell script. Should totally work. I'll try it again and see if maybe I just had some syntax error. – MattR Jul 20 '18 at 21:23

Convert repeated code into a loop

1 Answers1