Bash script that updates Intel e1000e driver

Question

I am now writing a small bash script that updates an Intel NIC driver to the latest version from the official website. Is there any way to improve\simplify the script? I want to avoid a lot of "if...else" stuff. Helpful tips will be appreciated!

#! /bin/bash
DRV_PKG_NAME="e1000e-3.0.4.tar.gz"
DRV_PKG_URL="http://downloadmirror.intel.com/15817/eng/e1000e-3.0.4.tar.gz"
# Downloading driver and extracting it to current directory
echo "Downloading and extracting driver package..."
if wget -q ${DRV_PKG_URL} && tar zxvf ${DRV_PKG_NAME} >/dev/null ; then
 echo "Done!"
else
 exit 1
fi
# Installing required build dependencies if necessary
echo "Installing build dependencies..."
if apt-get install -y build-essential linux-headers-$(uname -r) >/dev/null ; then
 echo "Done!"
else
 exit 1
fi
# Going into the driver source directory
cd e1000e-3.0.4/src/ 
# Building driver and updating initramfs
echo "Building module and updating initramfs..."
if { make install && update-initramfs -k all -u; } >/dev/null ; then
 echo "Done!"
else
 exit 1
fi 
echo "Purging unnecessary build dependencies..."
if apt-get -y purge build-essential >/dev/null ; then
 echo "Done!"
else
 exit 1
fi
# Restarting iface
echo "Restarting iface!"
{ ifdown eth0 && ifup eth0; } &>/dev/null
# Checking installed driver version
if [[ $(modinfo -F version e1000e) == "3.0.4-NAPI" ]] ; then
 echo "Driver succesfully installed!"
 exit 0
else 
 echo "Something Wrong...Try to re-install!"
 exit 1
fi

Answer 1

Simplifying the if-else that exit on failure

When the failure case of an if exits, you could move the success case out of the if, right after it, which will make it a bit shorter. And instead of an if, you could further simplify using the || operator. For example given this pattern:

if cmd; then
 echo "Done!"
else
 exit 1
fi

You could simplify as:

cmd || exit 1
echo "Done!"

You could apply this to all your ifs where you exit in case of failure.

But rather than just fail quietly, it would be better to do as @dopeghoti suggested and create a fail function and print some text in case of failure.

fail() {
 echo "$*"
 exit 1
}
cmd || fail 'Ouch, command failed'
echo "Done!"

Excessive comments

You have many obvious comments that add no value at all:

# Downloading driver and extracting it to current directory
echo "Downloading and extracting driver package..."
# Going into the driver source directory
cd e1000e-3.0.4/src/ 

In general, it's best when the code is self-explanatory. And in your script it's already the case, so you can remove all comments.

Error checking

Although you were careful to check errors in general, you forgot one:

cd e1000e-3.0.4/src/ 

Sure, if the tar earlier was successful, then this directory must exist, right? But what if one year later you update your script to get the package from e1000e-3.1.14.tar.gz instead and (heaven forbid) forget to update this directory name later in the script? If the cd command fails the script will happily continue and run the remaining commands, building whatever source you may have in the current directory instead of the right one. This will be safer:

cd e1000e-3.0.4/src/ || fail oopsie

Use more constants

It would be better to move more constants near the top of the file:

DRV_BASE_NAME=e1000e-3.0.4
DRV_SRC_DIR=$DRV_BASE_NAME/src
DRV_PKG_NAME=$DRV_BASE_NAME.tar.gz
DRV_PKG_URL=http://downloadmirror.intel.com/15817/eng/$DRV_PKG_NAME
MODULE_NAME=e1000e
MODULE_VERSION=3.0.4-NAPI

It will make the script easier to maintain.

Answer 2

If you want to avoid the if..else constructs you have strewn throughout the script (which is commendable), the best thing to do is to use a bash function with an appropriate name (e. g. fail) to do the thing you're doing throughout the script for you. My rule of thumb is that if I'm writing the same code more than thrice, I'm Doing It Wrong and something needs to be abstracted.

Also, bash has built-in mechanism for "did that thing I just tried work" tests, namely || chains, which execute the command after the || if the previous command has a nonzero exit code, as well as &&, which only executes the next command if the previous command has a zero exit code. For example, if I rewrite one of your if..else clauses thusly:

fail() {
 echo "$*"
 exit 1
}
# Example:
wget -q ${DRV_PKG_URL} && tar zxf ${DRV_PKG_NAME} || fail "Unable to download from ${DRV_PKG_URL}" && echo "Done!"

..if the wget succeeds, the call to fail is skipped, and we proceed to echo. If wget fails, we run call fail, which intrinsically exits the script after displaying the specified message.

• powerthrash: && echo "Done!" will not work properly in the following case: { make install && update-initramfs -k all -u; } >/dev/null || error_action && echo "Done!" echo will be executed even if the previous command fails.
• DopeGhoti: @powerthrash I think that can be fixed with set -o pipe fail up at the top of the script.

Answer 3

Here is simplified version of the script:

#! /bin/bash
DRV_PKG_NAME="e1000e-3.0.4.tar.gz"
DRV_PKG_URL="http://downloadmirror.intel.com/15817/eng/e1000e-3.0.4.tar.gz"
ERR_LOG="err.log"
# Redirecting all stderr 
exec 2>$ERR_LOG
# Function declaration
error_action () {
 echo "Failed!" 
 exit 1
}
# Downloading driver and extracting it to the current directory
echo "Downloading and extracting driver package..."
wget -q ${DRV_PKG_URL} && tar zxf ${DRV_PKG_NAME} || error_action
# Installing required build dependencies if necessary
echo "Installing build dependencies..."
apt-get install -y build-essential linux-headers-$(uname -r) >/dev/null || error_action
# Going into the driver source directory
cd e1000e-3.0.4/src/ 
# Building driver and updating initramfs
echo "Building module and updating initramfs..."
{ make install && update-initramfs -k all -u; } >/dev/null || error_action 
echo "Purging unnecessary build dependencies..."
apt-get -y purge build-essential >/dev/null || error_action
# Restarting iface
echo "Restarting iface..."
{ ifdown eth0 && ifup eth0; } &>/dev/null || error_action
# Checking installed driver version
if [[ $(modinfo -F version e1000e) == "3.0.4-NAPI" ]]; then
 echo "Driver successfully installed!"
else 
 echo "Something wrong... Try depmod -a or re-install again!"
 exit 1
fi
exit 0