[BASH] Critique my bash script? ( It's my first one )

tcoffeep · 2009-07-03 12:47:04

#!/bin/bash

if [ $# -eq 1 ]
then
    FILENAME=$1
    echo "Script Name : ${0}"
    echo "------------------"
    echo "Mounting CD on /media/cd"
    if [ ! -d "/media/cd" ]
    then
        sudo mkdir -pv /media/cd || echo "Do not have permissions!" ; exit
    fi
    mount -t iso9660 -o ro /dev/cdrom /media/cd
    echo "Mounting completed."
    echo "Ripping CD as : ${FILENAME}.iso"
    dd if=/dev/cdrom of=~/$FILENAME.iso
    echo "Unmounting CD."
    sudo umount /dev/cdrom || echo "Do not have permissions. " ; exit
    echo "Unmounting complete."
    echo "Shutting down script."
    exit
elif [ $# -eq 2 ]
then
    FILENAME=$1
    DIRECTORY=$2
    echo "Script Name : ${0}"
    echo "------------------"
    echo "Mounting CD on /media/cd"
    if [ ! -d "/media/cd" ]
    then
        sudo mkdir -pv /media/cd || echo "Do not have permissions!" ; exit
    fi
    mount -t iso9660 -o ro /dev/cdrom /media/cd
    echo "Ripping CD as : ${DIRECTORY}/${FILENAME}.iso"
    if [ -d "$DIRECTORY" ]
    then
        sudo dd if=/dev/cdrom of=$DIRECTORY/$FILENAME.iso || echo "Do not have permissions." ; exit
    elif [ ! -d "$DIRECTORY" ]
        then
        echo "Directory ${DIRECTORY} doesn't exist. Creating it now."
        mkdir -pv $DIRECTORY || echo "Do not have permissions!" ; exit
        sudo dd if=/dev/cdrom of=$DIRECTORY/$FILENAME.iso || echo "Do not have permissions." ; exit
    fi
    echo "Unmounting CD."
    sudo umount /dev/cdrom || echo "Do not have permissions!"; exit
    echo "Unmounting complete."
    echo "Shutting down script."
    exit
else
    echo "Script name : CD Ripper v0.3"
    echo ""
    echo "Help :"
    echo ""
    echo "./cdrip.sh <filename>"
    echo "---------------------"
    echo "No need to put ISO as it does this on its own. This rips the iso to your home location."
    echo ""
    echo "./cdrip.sh <filename> [directory]"
    echo "---------------------------------"
    echo "This rips the ISO to a directory specified by you."
    exit
fi

I'm really new to this, though I've made small things with perl and ruby. Never bash, though, however long I've been using Arch Linux. Please point out ways I could've done this better, and any possible things I might or should improve on?

[edit] : fixed a small error
[edit2] : added sudo's to the directory choice argument due to possible placing in root folders
[edit3] : fixed title spelling error

Last edited by tcoffeep (2009-07-03 15:31:22)

Procyon · 2009-07-03 13:44:40

if [ ! -d "/media/cd" ]
then
sudo mkdir -pv /media/cd || echo "Do not have permissions!" ; exit
fi
-----------
mkdir -pv $DIRECTORY || echo "Do not have permissions!" ; exit

You need braces around the two commands after || or the last one will be run even when the mkdir command succeeds.
e.g. mkdir -pv $DIRECTORY || (echo "Do not have permissions!" ; exit)

Although when mkdir fails it will already say why.

And you can also use "set -e" to make it quit after any (unchecked) command fails (see man bash)

Is it necessary to mount the CD when you just use dd on /dev/cdrom? Or is it just to make sure something is in there? Is there not a better way to do this?

And the usage is a bit awkward. It would be nicer to use absolute paths as the first argument. So
script.sh CD1 => script.sh ~/CD1.iso
script.sh CD1 . => script.sh CD1.iso
script.sh CD1 /new/path/CD1 => script.sh /new/path/CD1.iso
You can use dirname on the argument to see if it exists.

[ ! -d $(dirname $file) ] && mkdir -pv $(dirname $file)

tcoffeep · 2009-07-03 15:29:49

#!/bin/bash

if [ $# -eq 1 ]
then
    FILENAME=$1
    echo "Script Name : ${0}"
    echo "------------------"
    echo "Ripping CD as : ${FILENAME}.iso"
    sudo dd if=/dev/cdrom of=~/$FILENAME.iso
    echo "Shutting down script."
    exit
elif [ $# -eq 2 ]
then
    if [ "$2" = "." ]
    then
        FILENAME=$1
        echo "Script Name : ${0}"
        echo "------------------"
        echo "Ripping CD as : ${FILENAME}.iso"
        sudo dd if=/dev/cdrom of=$FILENAME.iso
        echo "Shutting down script."
        exit
    else
        FILENAME=$1
        DIRECTORY=$2
        echo "Script Name : ${0}"
        echo "------------------"
        echo "Ripping CD as : ${DIRECTORY}/${FILENAME}.iso"
        if [ -d "$DIRECTORY" ]
        then
            sudo dd if=/dev/cdrom of=$DIRECTORY/$FILENAME.iso
        elif [ ! -d "$DIRECTORY" ]
        then
            echo "Directory ${DIRECTORY} doesn't exist. Creating it now."
            mkdir -pv $DIRECTORY
            sudo dd if=/dev/cdrom of=$DIRECTORY/$FILENAME.iso
        fi
        echo "Shutting down script."
        exit
    fi
else
    echo "Script name : CD Ripper v0.4"
    echo ""
    echo "Help :"
    echo ""
    echo "cdrip.sh <filename>"
    echo "---------------------"
    echo "No need to put ISO as it does this on its own. This rips the iso to your home location."
    echo ""
    echo "cdrip.sh <filename> [directory]"
    echo "---------------------------------"
    echo "This rips the ISO to a directory specified by you."
    exit
fi

Something more akin to this?

Last edited by tcoffeep (2009-07-03 15:32:31)

Procyon · 2009-07-03 16:01:11

No, with the => lines I meant, (your script syntax) => (better absolute path syntax) but if you want to output to ~ by default with "basename directory" syntax then you should do that, but I thought it was weird.

Furthermore for better code you should rewrite the script/if-then-else so you only use 1 dd command (same applies to all the duplicate lines). Right now it will be really hard to maintain, and you don't want to use a ddcmd() function either!

For instance you can simplify it by

if [ $# -eq 2 ]; then DIR=$2; else DIR=~; fi
mkdir -pv $DIR #it's OK if it already exists
dd if=/dev/cdrom of="$DIR"/"$1".iso

Daenyth · 2009-07-04 01:23:41

One thing to note is that if the dvd has copy protection on it, dd will not make a correct copy.

tcoffeep · 2009-07-04 01:48:43

is there any way to kill the script and echo to terminal that there is copyright protection?

Arch Linux

#1 2009-07-03 12:47:04

[BASH] Critique my bash script? ( It's my first one )

#2 2009-07-03 13:44:40

Re: [BASH] Critique my bash script? ( It's my first one )

#3 2009-07-03 15:29:49

Re: [BASH] Critique my bash script? ( It's my first one )

#4 2009-07-03 16:01:11

Re: [BASH] Critique my bash script? ( It's my first one )

#5 2009-07-04 01:23:41

Re: [BASH] Critique my bash script? ( It's my first one )

#6 2009-07-04 01:48:43

Re: [BASH] Critique my bash script? ( It's my first one )

Board footer