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

• Pocketsand: Thanks for your detailed review, I completely agree with all your points. RE: your main gripe, I hacked the script together in part from this answer on askubuntu and I was thinking case 1ドル/2ドル in was doing some sort of erlang-like pattern matching/string unpacking but I realise now it is more like a switch statement and that the 1ドル and 2ドル are args passed into the script scope by systemd :)
• chicks: Gladly. :) 1ドル, 2ドル, etc. could be arguments in a shell function, but since you aren't doing that they are just the arguments to the whole script.