patternbashMinor
Bash script that updates Intel e1000e driver
Viewed 0 times
scriptintelupdatese1000ethatdriverbash
Problem
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
fiSolution
Simplifying the if-else that exit on failure
When the failure case of an
You could simplify as:
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
Excessive comments
You have many obvious comments that add no value at all:
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:
Sure, if the
Use more constants
It would be better to move more constants near the top of the file:
It will make the script easier to maintain.
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
fiYou 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 oopsieUse 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-NAPIIt will make the script easier to maintain.
Code Snippets
if cmd; then
echo "Done!"
else
exit 1
ficmd || exit 1
echo "Done!"fail() {
echo "$*"
exit 1
}
cmd || fail 'Ouch, command failed'
echo "Done!"# 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/cd e1000e-3.0.4/src/Context
StackExchange Code Review Q#42991, answer score: 5
Revisions (0)
No revisions yet.