Bash script with increasing disk write total

Question

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.

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.

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

Answer 1

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.