1

I wrote a bash script which reads the wifi usage in real time and writes it down to a file called 'usage' (in the $HOME directory) for every wifi the system connects to. The scripts works perfectly fine, but there are 2 things that bother me:

  1. It has an ever increasing "disk write total". Is this normal/expected since my script constantly updates the file usage in a while loop whenever my system is connected to a wifi. The total memory it uses is fixed to 496 KB. enter image description here

  2. It creates a lot of temporary files continously during the runtime of the script, which also gets deleted immediately. In the below image, note that files starting with sed* are those temporary files. As soon as I refresh that directory those files disappear and fresh new ones appear continously. enter image description here

Is there something wrong with my script or is it normal to expect such behaviour?

The script:

#!/bin/bash
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
filepath="$HOME/usage" #since cron jobs have a current working directory usually set as home
date_check() {
    if [ -f $filepath ]
    then 
        cur_date=$(date +%s)
        last_mod_date=$(date -r $filepath +%s)
        if [ $(date +%m) == 02 ] && [ $cycle_date -gt 28 ]
        then
            cycle_date=28
        fi
        cyc_date=$(date -d "$(date +%Y)-$(date +%m)-$cycle_date" +%s)
        if [[ $cyc_date -gt $last_mod_date ]] && [[ $cyc_date -lt $cur_date ]]
        then
            rm $filepath
        fi
    fi
}
wifi_record_update() {
    availability=""
    for dev in $net_interface
    do
        if [ $(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2) != "off/any" ]
        then
            availability="yes"
            ssid=$(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2)
            mac=$(iwconfig wlp0s20f3 | grep "Access Point" | tr -s ' ' | cut -d ' ' -f7)
            if grep -Fq "$mac" $filepath
            then
                used=$(grep "$mac" $filepath | cut -d ' ' -f3)
            else
                echo "$mac $ssid 0" >> $filepath
            fi
            break
        fi
    done
    sed -i "/off\/any/d" $filepath ###to delete garbage records which sometimes get collected with mac name set as off/any
}
###########    main
#####    identifying all wifi network adapter interfaces
net_interface=()
for dev in $(ls /sys/class/net/);
do
    if [ -d "/sys/class/net/$dev/wireless/" ]
    then
        net_interface+=("$dev")
    fi
done
##### getting cycle date
if [ -f $filepath ]
then
    cycle_date=$(head -n 1 $filepath)
else
    cycle_date=1 #default date at the start of first ever run of this script
fi
##### deletes $filepath file if the cycle date is passed
date_check;
if ! [[ -f $filepath ]]
then
    echo $cycle_date > $filepath
fi
##### main while loop
while true
do
    wifi_record_update
    while [ "$availability" != "" ]
    do
        prev=$cur
        cur=$(cat /sys/class/net/$dev/statistics/rx_bytes)
        add=$((cur-prev))
        echo "$(awk -v ad=$add -v ma=$mac '{if ($1==ma) {$3=$3+ad}; print $0 }' $filepath)" > $filepath ### updating the used value
        wifi_record_update
    done
done
exit 0

https://github.com/atul-g/bash_utility_scripts/blob/master/wifi_usage/wifi_usage.sh

2

1 Answer 1

2

Yes, there is something wrong with your shell script. Well, several things. Let's have a quick code review on the relevant function:

wifi_record_update() {
    availability=""
    for dev in $net_interface
    do
        if [ $(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2) != "off/any" ]
        then
            availability="yes"
            ssid=$(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2)
            mac=$(iwconfig wlp0s20f3 | grep "Access Point" | tr -s ' ' | cut -d ' ' -f7)
            if grep -Fq "$mac" $filepath
            then
                used=$(grep "$mac" $filepath | cut -d ' ' -f3)
            else
                echo "$mac $ssid 0" >> $filepath
            fi
            break
        fi
    done
    sed -i "/off\/any/d" $filepath ###to delete garbage records which sometimes get collected with mac name set as off/any
}

For future reference, the following review was applied to commit c65636db940235fd7458fb4c4432324401400658 of the script.

You are calling sed -i very often, which is why tons of temporary files are created (although removed later on).

Moreover, you use sed -i just to delete records which you shouldn't have written into the file in the first place.

Even more so, you already have code in place which is meant to prevent exactly those lines from being written:

if [ $(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2) != "off/any" ]
then
    ...
fi

Instead of adding that ugly sed -i hack, you should have investigated why the above mechanism doesn't work properly in the first place.

Now let's get to the actual problem of the script. You have established a typical race condition:

if [ $(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2) != "off/any" ]
then
    ...
    ssid=$(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2)
    ...
fi

You are asking twice for the SSID, and between those two lines, it could have changed. That's why you get those "off/any" lines despite your extra check. The proper solution is to fetch the SSID only once, and deal only with that one value:

ssid=$(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2)
if [ "$ssid" != off/any ]
then
    ...
fi

Oh, and by the way, you quoted the wrong side of the != comparison. There is no need to quote the harmless string "off/any", but there's every reason to quote the result of a sub shell invocation "$(...)". And also, to quote every variable expansion, which you see after my change.

To avoid this and similar typical quoting mistakes, I strongly suggest to run your script through the excellent shellcheck tool. Please do this for every shell script you ever write before using it on production (and before using it for anything at all, while we are at it):

shellcheck YOUR-SCRIPT.sh

But there's more! After those corrections, there remains a second race condition which will generate incorrect ssid/mac combinations from time to time. The reason is exactly the same: You are asking twice for the iwconfig result, at different points in time, during which the access point may have changed:

ssid=$(iwconfig wlp0s20f3 | grep ESSID | cut -d: -f2)
if [ "$ssid" != off/any ]
then
    ...
    mac=$(iwconfig wlp0s20f3 | grep "Access Point" | tr -s ' ' | cut -d ' ' -f7)
    ...
fi

The proper solution here is to fetch the whole iwconfig output only once, and to extract the SSID and MAC both from exactly the same point in time. Since the output of iwconfig is relatively small, let's just use a variable instead of creating a file. Moreover, note that shell variables may contain multiple lines, which are properly reproduced by echo as long as you quote correctly (i.e. echo "$iwconfig_output" instead of echo $iwconfig_output):

iwconfig_output=$(iwconfig wlp0s20f3)
ssid=$(echo "$iwconfig_output" | grep ESSID | cut -d: -f2)
if [ "$ssid" != off/any ]
then
    ...
    mac=$(echo "$iwconfig_output" | grep "Access Point" | tr -s ' ' | cut -d ' ' -f7)
    ...
fi

There are probably more issues in that script, so I strongly advise to apply the same analysis of race conditions and proper quoting to the rest of the script as well.

6
  • Wow, first off, thanks a lot for taking your time off and helping me out here, really appreciate it! I will update my code as per your instructions. I think I should have mentioned it in the question that I'm pretty new to shell scripting and should have reviewed my code more. Thanks a lot for your help! :D Apr 28, 2020 at 15:54
  • I did update my code as you told and also understood the importance of variable expansion. But there is this one thing that is bothering me, as you said I need to have quotes around my $ssid in the if condition which I noticed wasn't working for me for some reason. Here is a picture of a simple if condition I tried with quoted variable expansion and strangely enough, it was behaving in an unexpected way: imgur.com/SjyulmK Apr 29, 2020 at 6:33
  • There's a minor bug in your "cut" mechanism. You are splitting a string like "ESSID: off/any" at ":", leaving a leading space in the second part. With proper escaping, the shell now compares "space + off/any" to "off/any", which are of course unequal. By the way, proper string comparison is done by "=", not "==".
    – vog
    Apr 29, 2020 at 18:12
  • sorry, but I'm not able to quite get you, there doesn't seem to be any space between ":" and "off/any" in the original iwconfig output. When I run this command: echo "$iw_rep" | grep "ESSID" - the output I get is: "wlp0s20f3 IEEE 802.11 ESSID:off/any". As you can see there isn't any space between the colon and off/any. Apr 30, 2020 at 16:07
  • In that case I propose to add a debug output like `echo "#$ssid#", i.e. two markers around the string. Then observe the output and see if there is any whitespace that could explain the difference. Besides, this seems to be a separate issue and might warrant a new question (but feel free to link to it from here, if you like)
    – vog
    Apr 30, 2020 at 19:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.