Shell script that verifies checksums when moving between filesystems

Question

I coded this because it seemed like a fun project, would be awesome if someone reviewed this.

Thanks!

#! /bin/sh


#function that checks if dependencies are installed
check_software() {
    #check if md5sum is installed

    if [ $(command -v md5sum) > /dev/null 2>&1 ]; then
        echo 'md5sum'

    #check if md5 is installed
    elif [ $(command -v md5) > /dev/null 2>&1 ]; then
        echo 'md5'

    #if neither are installed, quit
    else 
        echo "Neither md5sum nor md5 are installed. Quitting." 1>&2
        exit 1
    fi
}

#function to check if we're moving to same filesystem type
check_filesystem() {
    #df gives us detailed information about the file, use awk to get only the disk
    DISK1=$(df "$SOURCE" | tail -1 | head -1 | awk '{print $1}')

    #run dirname to remove the last bit of the path as the file does not exist yet
    DISK2=$(df $(dirname "$DESTINATION") | tail -1 | head -1 | awk '{print $1}')

    if [ "$DISK1" == "$DISK2" ]; then
        echo 1
    else
        echo 0
    fi
}

main() {
     CHECKSOFT=$(check_software)

     #if file is being moved on the same filesystem
     if [ "$(check_filesystem)" == "1" ]; then
        #move normally
        if [ "$FLAGS" != '' ]; then
            mv "-""$FLAGS" "$SOURCE" "$DESTINATION"
        else
            mv "$SOURCE" "$DESTINATION"
        fi
     else
        echo "Starting bettermv"
        #copy the file
        cp -p "$SOURCE" "$DESTINATION"

        #this bit of code extracts only the hashes from the output produced
        if [ "$CHECKSOFT" = "md5sum" ]; then
            CHECKSUMSOURCE="$($CHECKSOFT "$SOURCE" | awk '{print $1}')"
            CHECKSUMDEST="$($CHECKSOFT "$DESTINATION" | awk '{print $1}')"
        else
            CHECKSUMSOURCE="$($CHECKSOFT "$SOURCE" | awk '{print $4}')"
            CHECKSUMDEST="$($CHECKSOFT "$DEST" | awk '{print $4}')"
        fi

        #compare checksums and if they match up
        if [ "$CHECKSUMSOURCE" == "$CHECKSUMDEST" ]; then
            #remove original and exit
            rm "$SOURCE"
            echo "Move completed successfuly"
            return 0
        #if they don't match up    
        else
            #display error message and quit
            echo "Checksums did not match, please try again" 1>&2
            rm "$DESTINATION"
            exit 1
        fi
    fi
}

FLAGS=''
#get all the move flags and store them in a variable
while getopts "finv" opt; do
    case $opt in 
        f)
        FLAGS="$FLAGS""f"
        ;;
        i)
        FLAGS="$FLAGS""i"
        ;;
        n)
        FLAGS="$FLAGS""n"
        ;;
        v)
        FLAGS="$FLAGS""v"
        ;;
        \?)
        echo "Invalid argument $OPTARG"
        ;;
    esac
done

#shift optindex past the flags
shift $(( OPTIND-1 ))

#iterate through files to move and move them to the destination (last argument supplied)
for i in $@; do
    if [ $i != ${BASH_ARGV[0]} ]; then
        SOURCE=$i
        DESTINATION="${BASH_ARGV[0]}""$(basename $SOURCE)"
        main

    fi
done
exit 0

Answer 1

I'll start with your entry point and will go through you code step by step. The last code block will contain an updated version.

The key elements in my changes are split your functions and only do one specific task, e.g. extra function for getting a checksum of a file and another one comparing two checksums etc. A function should also get all necessary information by PARAMETER not by an environment variable set outside. This simplifies testing a lot.

The getopts part is fine but you should just store the flag with the additional "-" as it will simplify things later in your code. You may also want to exit if an unknown parameter was provided. I would also rename FLAGS to MV_FLAGS as it describes better what the flags are for. You would normally also print a usage if an invalid parameter was provided. After parsing the options you should also check if enough parameters were provided and print a message otherwise but this is just a UX thing.

MV_FLAGS=''
#get all the move flags and store them in a variable
while getopts "finv" opt; do
    case $opt in
        f) MV_FLAGS="$MV_FLAGS"f ;;
        i) MV_FLAGS="$MV_FLAGS"i ;;
        n) MV_FLAGS="$MV_FLAGS"n ;;
        v) MV_FLAGS="$MV_FLAGS"v ;;
        ?)
        echo "Invalid argument $1" >&2
        exit 1
        ;;
    esac
done
[ -n "$MV_FLAGS" ] && MV_FLAGS="-$MV_FLAGS"

#shift optindex past the flags
shift $(( OPTIND-1 ))

if [ $# -lt 1 ] ; then
   echo "You must specify additional filenames" >&2
   exit 1
fi

For the next part you should definitely quote $@ as otherwise you won't handle parameters with spaces correctly. There is also no need to compare $i with ${BASH_ARGV[0]} as $@ contains the parameter starting at 1. Instead of specifying environment variables to call your main function you should specify them as a parameter instead. BASH_ARGV is according to man bash also only set if you have debugging enabled, so it is not portable. I also don't really understand what DESTINATION should be, but you should change it as well. You should also either quote your parameters with " or use ${ I wouldn't mix them and there is no need to use " as well as ${}. You should also check if the source file is a valid file before pushing calling your function. The function name could also be improved, e.g move_file

for i in "$@" ; do
    SOURCE="$i"
    DESTINATION="$0""$(basename "$SOURCE")"
    if [ ! -f "$SOURCE" ] ; then
       echo "$SOURCE does not exist, ignoring." >&2
       continue
    fi
     move_file "$SOURCE" "$DESTINATION"
done

In your main and my move_file function you always call check_software but there is no need for it. It is enough to call it once at the beginning. You should also create a function handling verifying the filenames so your code doesn't get clustered with the different program parameters etc:

#function that checks if dependencies are installed
check_software() {
    for name in md5sum md5 openssl ; do
        if [ $(type "$name") > /dev/null 2>&1 ] ; then
           return 0
        fi
    done
    return 1
}

if ! check_software ; then
    echo "Neither md5sum, md5 nor openssl are installed. Quitting." >&2
    exit 1
fi

Your check_filesystem can also be simplified a lot if you use a device number instead of parsing df, I'd also rename it to same_filesystem

same_filesystem() {
    if [ $# -ne 2 ]  ; then
       echo "Usage: $0 source target" >&2
       return 1
    fi
    FS1=$(stat -c "%d" "$1")
    FS2=$(stat -c "%d" "$(dirname "$2")" )
    [ "$FS1" -eq "$FS2" ]
    return $?
}

You can also simplify your main/move_file function a bit, e.g:

move_file() {
     # we already checked if the necessary programs are available no
     # need to check again
     if [ $# -ne 2 ] ; then
         echo "Usage: $0 source target" >&2
         return 1
     fi

     SOURCE="$1"
     DESTINATION="$2"

     if [ -f "$DESTINATION" ] ; then
       # do something if file already exists?!
       :
     fi

     #if file is being moved on the same filesystem
     if same_filesystem "$SOURCE" "$DESTINATION" ; then
        mv $MV_FLAGS -- "$SOURCE" "$DESTINATION"
        return 0
     fi

     echo "Starting bettermv" >&2
     #copy the file
     cp -p "$SOURCE" "$DESTINATION"

     if verify_files "$SOURCE" "$DESTINATION" ; then
        rm "$SOURCE"
        return 0
    else
        echo "Error moving $SOURCE to $DESTINATION, abort." >&2
        rm "$DESTINATION"
        return 1
     fi
}

get_checksum() {
     if [ $# -ne 1 ] ; then
         echo "Usage: $0 source" >&2
         return 1
     fi
     if type openssl >/dev/null 2>&1 ; then
         echo $(openssl md5 "$1" | awk '{print $2}')
     elif type md5sum >/dev/null 2>&1 ; then
         echo $(md5sum "$1" | awk '{print $1}')
     elif type md5 >/dev/null 2>&1 ; then
         echo $(md5 "$1" | awk '{print $4}')
     fi
     return 1
}

verify_files() {
    if [ $# -ne 2 ] ; then
         echo "Usage: $0 source target" >&2
         return 1
     fi

    CHECKSUM1=$(get_checksum "$1")
    CHECKSUM2=$(get_checksum "$2")

    [ $CHECKSUM1 = $CHECKSUM2 ]
    return $?
}

The complete script with all changes (i tested it):

#!/bin/sh

#function that checks if dependencies are installed
check_software() {
  for name in md5sum md5 openssl ; do
  if type "$name" 1>/dev/null 2>&1 ; then
    return 0
  fi
  done
  return 1
}

if ! check_software ; then
  echo "Neither md5sum, md5 nor openssl are installed. Quitting." >&2
  exit 1
fi


move_file() {
  # we already checked if the necessary programs are available no
  # need to check again
  if [ $# -ne 2 ] ; then
    echo "Usage: $0 source target" >&2
    return 1
  fi

  SOURCE="$1"
  DESTINATION="$2"

  if [ -f "$DESTINATION" ] ; then
    # do something if file already exists?!
    :
  fi

  #if file is being moved on the same filesystem
  if same_filesystem "$SOURCE" "$DESTINATION" ; then
    mv $MV_FLAGS -- "$SOURCE" "$DESTINATION"
    return 0
  fi

  echo "Starting bettermv" >&2

  #copy the file
  cp -p "$SOURCE" "$DESTINATION"

  if verify_files "$SOURCE" "$DESTINATION" ; then
    rm "$SOURCE"
    return 0
  else
    echo "Error moving $SOURCE to $DESTINATION, abort." >&2
    rm "$DESTINATION"
    return 1
  fi
}


same_filesystem() {
  if [ $# -ne 2 ]  ; then
    echo "Usage: $0 source target" >&2
    return 1
  fi

  FS1=$(stat -c "%d" "$1")
  FS2=$(stat -c "%d" "$(dirname "$2")" )

  [ "$FS1" -eq "$FS2" ]
  return $?
}


get_checksum() {
  if [ $# -ne 1 ] ; then
    echo "Usage: $0 source" >&2
    return 1
  fi

  if type md5sum >/dev/null 2>&1 ; then
    echo $(md5sum -- "$1" | awk '{print $1}')
  elif type md5 >/dev/null 2>&1 ; then
    echo $(md5 "$1" | awk '{print $4}')
  elif type openssl >/dev/null 2>&1 ; then
    echo $(openssl md5 "$1" | awk '{print $2}')
  else
    return 1
  fi
}


verify_files() {
  if [ $# -ne 2 ] ; then
    echo "Usage: $0 source target" >&2
    return 1
  fi

  CHECKSUM1=$(get_checksum "$1")
  CHECKSUM2=$(get_checksum "$2")

  [ $CHECKSUM1 = $CHECKSUM2 ]
  return $?
}



MV_FLAGS=''
#get all the move flags and store them in a variable
while getopts "finv" opt; do
  case $opt in
  f) MV_FLAGS="$MV_FLAGS"f ;;
  i) MV_FLAGS="$MV_FLAGS"i ;;
  n) MV_FLAGS="$MV_FLAGS"n ;;
  v) MV_FLAGS="$MV_FLAGS"v ;;
  ?)
  echo "Invalid argument $1" >&2
  exit 1
  ;;
  esac
done

[ -n "$MV_FLAGS" ] && MV_FLAGS="-$MV_FLAGS"

#shift optindex past the flags
shift $(( OPTIND-1 ))

if [ $# -lt 1 ] ; then
  echo "You must specify additional filenames" >&2
  exit 1
fi

for i in "$@" ; do
  SOURCE="$i"
  DESTINATION="$0""$(basename "$SOURCE")"

  if [ ! -f "$SOURCE" ] ; then
    echo "$SOURCE does not exist, ignoring." >&2
    continue
  fi

  move_file "$SOURCE" "$DESTINATION"
done