HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Bash script that updates Intel e1000e driver

Submitted by: @import:stackexchange-codereview··
0
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
fi

Solution

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.

Code Snippets

if cmd; then
    echo "Done!"
else
    exit 1
fi
cmd || 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.