Restart network-manager service if connection down on wake shell script

Question

I use Ubuntu (Trusty and Xenial) and usually put my computer in sleep mode rather than power it down. A common problem is that upon wake the network is down so one has to restart the network-manager service. E.g.

sudo service network-manager restart

I am tired of doing this so I have hacked the following together:

#!/bin/sh

# should live in /lib/systemd/system-sleep/...
# sudo mv restart-network-on-wake.sh /lib/systemd/system-sleep/
# sudo chmod a+x /lib/systemd/system-sleep/restart-network-on-wake.sh

case $1/$2 in
    pre/*)
        echo "Going to $2..."
        exit 0
        ;;
    post/*)
        echo "Waking up from $2..."
        if ! ping -q -c 1 -W 1 8.8.8.8 > /dev/null; then
            echo "IPv4 is down, restarting network-manager"
            service network-manager restart
        fi
        ;;
esac

I am a novice at shell scripting I guess and don't have a strong understanding of Linux internals. Do you have any thoughts?

Answer 1

Welcome to Code Review. Thanks for asking for a review. Overall I think you did a great job, even for an experienced scripter.

good things

• consistent indentation
• helpful comments about installation
• you didn't forget the #! line
• using if to check for return values without creating extra variables
• I'm a fan of bash and prefer it in most cases, but specifying an older shell will help this be portable to environment without bash like embedded systems.

suggestions

• include a comment explaining what the purpose of the script is. The file name is pretty descriptive, but someone might rename it accidentally. So including the filename in the comment would not be too redundant.
• run shell code through shellcheck. Lucky for you it didn't find any issues with this code
• it is a good practice to include a *) in your case to catch the unexpected.
• I like for there to be a space before the ) in the case matches and a blank line after the ;;. The space before the ) makes it clearer what is part of the pattern and what isn't. case statements can get longish so having the extra space after the ;; may annoy you if you have a small screen, but I find it makes it easier to follow the flow when it is more separated.

main gripe

Why did you include $2 in your case? Since you didn't cross a function call boundary there's nothing in the case statement that will prevent access from $2 inside. I've integrated these suggestions into a rewrite I haven't tested. But it should work and seems cleaner to me:

#!/bin/sh

# restart-network-on-wake.sh - systemd/Ubuntu helper

# INSTALLATION
#
# The script should live in /lib/systemd/system-sleep/...
#   sudo mv restart-network-on-wake.sh /lib/systemd/system-sleep/
#   sudo chmod a+x /lib/systemd/system-sleep/restart-network-on-wake.sh

case $1 in
    pre )
        echo "Going to $2..."
        exit 0
        ;;

    post )
        echo "Waking up from $2..."
        if ! ping -q -c 1 -W 1 8.8.8.8 > /dev/null; then
            echo "IPv4 is down, restarting network-manager"
            service network-manager restart
        fi
        ;;

    * )
        echo "ERROR: got $1 which is not expected"
        exit 1
        ;;
esac