current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

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
share|improve this question
add comment

2 Answers

up vote 1 down vote

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.

share|improve this answer
    
Welcome to Code Review ! I see that you've modified the script, but could you include a bit of explanation about what you've changed and why ? Part of a review is explaining why we would change something. –  Marc-Andre Feb 28 at 19:11
1  
It is more helpful to explain what you are doing here, what techniques you are using, rather than just dumping code. It seems to be a nice way, but it could use a short explanation. –  Simon André Forsberg Feb 28 at 19:13
    
Hello, and thanks for the welcome. I have added a more detailed explanation of the whys and hows of my suggested answer. I hope this is more useful. –  DopeGhoti Feb 28 at 19:18
    
&& 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. –  powerthrash Feb 28 at 20:14
    
@powerthrash I think that can be fixed with set -o pipe fail up at the top of the script. –  DopeGhoti Feb 28 at 21:47
add comment
up vote 1 down vote

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
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.