Backup with Hanoi Strategy

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








6












$begingroup$


I want your suggestions about this shell script. I use it to backup some configurations on my home media server.



What improvements would you do? Keep in mind, I want to optimize space and speed and at the same time, backup the maximum configurations over time.



p.-s. For more informations about the hanoi scheme: https://en.wikipedia.org/wiki/Backup_rotation_scheme.



ideas :



  • Only use the hanoi scheme when the limit space is reach by checking the space required by rsync.

#!/bin/bash

# ==================================================================================
# Description: BackUp script following a Hanoi scheme using rsync.
# Author: Hugo Lapointe Di Giacomo
# ==================================================================================

set -o errexit
set -o nounset
#set -o xtrace

# ----------------------------------------------------------------------------------
# Options
# ----------------------------------------------------------------------------------
setsMax=16 # ................................ Maximum number of sets bakcup.
cmdPrefix="" # .............................. "echo" if dry-run enable, "" otherwise.
srcDir="/appdata" # ......................... Source directory to backup.
dstDir="/mnt/backup/snapshots" # ............ Destination directory to backup in.
logDir="/mnt/backup/logs" # ................. Logs directory.


# ----------------------------------------------------------------------------------
# Constants
# ----------------------------------------------------------------------------------
SEC_PER_DAY=86400; # ........................ Number of seconds per day.
LATEST_SNAP="$dstDir/latest"; # ............. The "latest" snap.
LATEST_LOG="$logDir/latest"; # .............. The "latest" log.
DATE_FORMAT="%Y-%m-%d"; # ................... The format of a date.
HOUR_FORMAT="%H:%M"; # ...................... The format of a hour.


# ----------------------------------------------------------------------------------
# Show usage of this script.
# ----------------------------------------------------------------------------------
function showUsage()
echo "Usage: $0 [OPTIONS]..."
echo ""
echo "Options available:"
echo " -h, --help Show usage of the script."
echo " -m, --max Maximum sets of backup."
echo " -s, --src Source directory to backup."
echo " -d. --dst Destination directory to backup in."
echo " -l, --log Log directory."
echo " -r, --dry Enable dry-run."



# ----------------------------------------------------------------------------------
# Parse arguments.
# ----------------------------------------------------------------------------------
function parseArgs() --max)
setsMax=$2:-$setsMax
shift 2
;;

-s


# ----------------------------------------------------------------------------------
# Create a backup with rsync.
# ----------------------------------------------------------------------------------
function createBackUp() tee "$logFile"


# ----------------------------------------------------------------------------------
# Update the latest links.
# ----------------------------------------------------------------------------------
function updateLatestLinks()
$cmdPrefix rm -f "$LATEST_SNAP"
$cmdPrefix ln -s "$snapDir" "$LATEST_SNAP"

$cmdPrefix rm -f "$LATEST_LOG"
$cmdPrefix ln -s "$logFile" "$LATEST_LOG"


# ----------------------------------------------------------------------------------
# Calculate the previous move in the cycle.
# https://en.wikipedia.org/wiki/Backup_rotation_scheme
# ----------------------------------------------------------------------------------
function calculateDaysFromBackupOfSameSet()
local todayInSec=$(date +%s)
local todayInDay=$(($todayInSec / $SEC_PER_DAY))

local daysToBackup=$((2 ** ($setsMax - 1)))

local daysElapsed=0
local i=1

while [ $i -lt $(($daysToBackup / 2)) ]; do
local rotation=$(($todayInDay & $i))

if [ $rotation -eq 0 ]; then
daysElapsed=$(($i * 2))
break
fi

daysElapsed=$(($daysToBackup / 2))
i=$((2 * $i))
done

echo $daysElapsed



# ----------------------------------------------------------------------------------
# Main function.
# ----------------------------------------------------------------------------------
function main()
parseArgs $@

local todayDatetime=$(date +$DATE_FORMAT.$HOUR_FORMAT)

if createBackUp $todayDatetime; then
updateLatestLinks

local daysElapsed=$(calculateDaysFromBackupOfSameSet)
local expiredDay=$(date -d "$daysElapsed days ago" +$DATE_FORMAT)
$cmdPrefix rm -frv "$dstDir/$expiredDay."*
fi


main $@









share|improve this question











$endgroup$











  • $begingroup$
    I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
    $endgroup$
    – Zeta
    Mar 15 at 19:19











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    Mar 15 at 19:25

















6












$begingroup$


I want your suggestions about this shell script. I use it to backup some configurations on my home media server.



What improvements would you do? Keep in mind, I want to optimize space and speed and at the same time, backup the maximum configurations over time.



p.-s. For more informations about the hanoi scheme: https://en.wikipedia.org/wiki/Backup_rotation_scheme.



ideas :



  • Only use the hanoi scheme when the limit space is reach by checking the space required by rsync.

#!/bin/bash

# ==================================================================================
# Description: BackUp script following a Hanoi scheme using rsync.
# Author: Hugo Lapointe Di Giacomo
# ==================================================================================

set -o errexit
set -o nounset
#set -o xtrace

# ----------------------------------------------------------------------------------
# Options
# ----------------------------------------------------------------------------------
setsMax=16 # ................................ Maximum number of sets bakcup.
cmdPrefix="" # .............................. "echo" if dry-run enable, "" otherwise.
srcDir="/appdata" # ......................... Source directory to backup.
dstDir="/mnt/backup/snapshots" # ............ Destination directory to backup in.
logDir="/mnt/backup/logs" # ................. Logs directory.


# ----------------------------------------------------------------------------------
# Constants
# ----------------------------------------------------------------------------------
SEC_PER_DAY=86400; # ........................ Number of seconds per day.
LATEST_SNAP="$dstDir/latest"; # ............. The "latest" snap.
LATEST_LOG="$logDir/latest"; # .............. The "latest" log.
DATE_FORMAT="%Y-%m-%d"; # ................... The format of a date.
HOUR_FORMAT="%H:%M"; # ...................... The format of a hour.


# ----------------------------------------------------------------------------------
# Show usage of this script.
# ----------------------------------------------------------------------------------
function showUsage()
echo "Usage: $0 [OPTIONS]..."
echo ""
echo "Options available:"
echo " -h, --help Show usage of the script."
echo " -m, --max Maximum sets of backup."
echo " -s, --src Source directory to backup."
echo " -d. --dst Destination directory to backup in."
echo " -l, --log Log directory."
echo " -r, --dry Enable dry-run."



# ----------------------------------------------------------------------------------
# Parse arguments.
# ----------------------------------------------------------------------------------
function parseArgs() --max)
setsMax=$2:-$setsMax
shift 2
;;

-s


# ----------------------------------------------------------------------------------
# Create a backup with rsync.
# ----------------------------------------------------------------------------------
function createBackUp() tee "$logFile"


# ----------------------------------------------------------------------------------
# Update the latest links.
# ----------------------------------------------------------------------------------
function updateLatestLinks()
$cmdPrefix rm -f "$LATEST_SNAP"
$cmdPrefix ln -s "$snapDir" "$LATEST_SNAP"

$cmdPrefix rm -f "$LATEST_LOG"
$cmdPrefix ln -s "$logFile" "$LATEST_LOG"


# ----------------------------------------------------------------------------------
# Calculate the previous move in the cycle.
# https://en.wikipedia.org/wiki/Backup_rotation_scheme
# ----------------------------------------------------------------------------------
function calculateDaysFromBackupOfSameSet()
local todayInSec=$(date +%s)
local todayInDay=$(($todayInSec / $SEC_PER_DAY))

local daysToBackup=$((2 ** ($setsMax - 1)))

local daysElapsed=0
local i=1

while [ $i -lt $(($daysToBackup / 2)) ]; do
local rotation=$(($todayInDay & $i))

if [ $rotation -eq 0 ]; then
daysElapsed=$(($i * 2))
break
fi

daysElapsed=$(($daysToBackup / 2))
i=$((2 * $i))
done

echo $daysElapsed



# ----------------------------------------------------------------------------------
# Main function.
# ----------------------------------------------------------------------------------
function main()
parseArgs $@

local todayDatetime=$(date +$DATE_FORMAT.$HOUR_FORMAT)

if createBackUp $todayDatetime; then
updateLatestLinks

local daysElapsed=$(calculateDaysFromBackupOfSameSet)
local expiredDay=$(date -d "$daysElapsed days ago" +$DATE_FORMAT)
$cmdPrefix rm -frv "$dstDir/$expiredDay."*
fi


main $@









share|improve this question











$endgroup$











  • $begingroup$
    I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
    $endgroup$
    – Zeta
    Mar 15 at 19:19











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    Mar 15 at 19:25













6












6








6





$begingroup$


I want your suggestions about this shell script. I use it to backup some configurations on my home media server.



What improvements would you do? Keep in mind, I want to optimize space and speed and at the same time, backup the maximum configurations over time.



p.-s. For more informations about the hanoi scheme: https://en.wikipedia.org/wiki/Backup_rotation_scheme.



ideas :



  • Only use the hanoi scheme when the limit space is reach by checking the space required by rsync.

#!/bin/bash

# ==================================================================================
# Description: BackUp script following a Hanoi scheme using rsync.
# Author: Hugo Lapointe Di Giacomo
# ==================================================================================

set -o errexit
set -o nounset
#set -o xtrace

# ----------------------------------------------------------------------------------
# Options
# ----------------------------------------------------------------------------------
setsMax=16 # ................................ Maximum number of sets bakcup.
cmdPrefix="" # .............................. "echo" if dry-run enable, "" otherwise.
srcDir="/appdata" # ......................... Source directory to backup.
dstDir="/mnt/backup/snapshots" # ............ Destination directory to backup in.
logDir="/mnt/backup/logs" # ................. Logs directory.


# ----------------------------------------------------------------------------------
# Constants
# ----------------------------------------------------------------------------------
SEC_PER_DAY=86400; # ........................ Number of seconds per day.
LATEST_SNAP="$dstDir/latest"; # ............. The "latest" snap.
LATEST_LOG="$logDir/latest"; # .............. The "latest" log.
DATE_FORMAT="%Y-%m-%d"; # ................... The format of a date.
HOUR_FORMAT="%H:%M"; # ...................... The format of a hour.


# ----------------------------------------------------------------------------------
# Show usage of this script.
# ----------------------------------------------------------------------------------
function showUsage()
echo "Usage: $0 [OPTIONS]..."
echo ""
echo "Options available:"
echo " -h, --help Show usage of the script."
echo " -m, --max Maximum sets of backup."
echo " -s, --src Source directory to backup."
echo " -d. --dst Destination directory to backup in."
echo " -l, --log Log directory."
echo " -r, --dry Enable dry-run."



# ----------------------------------------------------------------------------------
# Parse arguments.
# ----------------------------------------------------------------------------------
function parseArgs() --max)
setsMax=$2:-$setsMax
shift 2
;;

-s


# ----------------------------------------------------------------------------------
# Create a backup with rsync.
# ----------------------------------------------------------------------------------
function createBackUp() tee "$logFile"


# ----------------------------------------------------------------------------------
# Update the latest links.
# ----------------------------------------------------------------------------------
function updateLatestLinks()
$cmdPrefix rm -f "$LATEST_SNAP"
$cmdPrefix ln -s "$snapDir" "$LATEST_SNAP"

$cmdPrefix rm -f "$LATEST_LOG"
$cmdPrefix ln -s "$logFile" "$LATEST_LOG"


# ----------------------------------------------------------------------------------
# Calculate the previous move in the cycle.
# https://en.wikipedia.org/wiki/Backup_rotation_scheme
# ----------------------------------------------------------------------------------
function calculateDaysFromBackupOfSameSet()
local todayInSec=$(date +%s)
local todayInDay=$(($todayInSec / $SEC_PER_DAY))

local daysToBackup=$((2 ** ($setsMax - 1)))

local daysElapsed=0
local i=1

while [ $i -lt $(($daysToBackup / 2)) ]; do
local rotation=$(($todayInDay & $i))

if [ $rotation -eq 0 ]; then
daysElapsed=$(($i * 2))
break
fi

daysElapsed=$(($daysToBackup / 2))
i=$((2 * $i))
done

echo $daysElapsed



# ----------------------------------------------------------------------------------
# Main function.
# ----------------------------------------------------------------------------------
function main()
parseArgs $@

local todayDatetime=$(date +$DATE_FORMAT.$HOUR_FORMAT)

if createBackUp $todayDatetime; then
updateLatestLinks

local daysElapsed=$(calculateDaysFromBackupOfSameSet)
local expiredDay=$(date -d "$daysElapsed days ago" +$DATE_FORMAT)
$cmdPrefix rm -frv "$dstDir/$expiredDay."*
fi


main $@









share|improve this question











$endgroup$




I want your suggestions about this shell script. I use it to backup some configurations on my home media server.



What improvements would you do? Keep in mind, I want to optimize space and speed and at the same time, backup the maximum configurations over time.



p.-s. For more informations about the hanoi scheme: https://en.wikipedia.org/wiki/Backup_rotation_scheme.



ideas :



  • Only use the hanoi scheme when the limit space is reach by checking the space required by rsync.

#!/bin/bash

# ==================================================================================
# Description: BackUp script following a Hanoi scheme using rsync.
# Author: Hugo Lapointe Di Giacomo
# ==================================================================================

set -o errexit
set -o nounset
#set -o xtrace

# ----------------------------------------------------------------------------------
# Options
# ----------------------------------------------------------------------------------
setsMax=16 # ................................ Maximum number of sets bakcup.
cmdPrefix="" # .............................. "echo" if dry-run enable, "" otherwise.
srcDir="/appdata" # ......................... Source directory to backup.
dstDir="/mnt/backup/snapshots" # ............ Destination directory to backup in.
logDir="/mnt/backup/logs" # ................. Logs directory.


# ----------------------------------------------------------------------------------
# Constants
# ----------------------------------------------------------------------------------
SEC_PER_DAY=86400; # ........................ Number of seconds per day.
LATEST_SNAP="$dstDir/latest"; # ............. The "latest" snap.
LATEST_LOG="$logDir/latest"; # .............. The "latest" log.
DATE_FORMAT="%Y-%m-%d"; # ................... The format of a date.
HOUR_FORMAT="%H:%M"; # ...................... The format of a hour.


# ----------------------------------------------------------------------------------
# Show usage of this script.
# ----------------------------------------------------------------------------------
function showUsage()
echo "Usage: $0 [OPTIONS]..."
echo ""
echo "Options available:"
echo " -h, --help Show usage of the script."
echo " -m, --max Maximum sets of backup."
echo " -s, --src Source directory to backup."
echo " -d. --dst Destination directory to backup in."
echo " -l, --log Log directory."
echo " -r, --dry Enable dry-run."



# ----------------------------------------------------------------------------------
# Parse arguments.
# ----------------------------------------------------------------------------------
function parseArgs() --max)
setsMax=$2:-$setsMax
shift 2
;;

-s


# ----------------------------------------------------------------------------------
# Create a backup with rsync.
# ----------------------------------------------------------------------------------
function createBackUp() tee "$logFile"


# ----------------------------------------------------------------------------------
# Update the latest links.
# ----------------------------------------------------------------------------------
function updateLatestLinks()
$cmdPrefix rm -f "$LATEST_SNAP"
$cmdPrefix ln -s "$snapDir" "$LATEST_SNAP"

$cmdPrefix rm -f "$LATEST_LOG"
$cmdPrefix ln -s "$logFile" "$LATEST_LOG"


# ----------------------------------------------------------------------------------
# Calculate the previous move in the cycle.
# https://en.wikipedia.org/wiki/Backup_rotation_scheme
# ----------------------------------------------------------------------------------
function calculateDaysFromBackupOfSameSet()
local todayInSec=$(date +%s)
local todayInDay=$(($todayInSec / $SEC_PER_DAY))

local daysToBackup=$((2 ** ($setsMax - 1)))

local daysElapsed=0
local i=1

while [ $i -lt $(($daysToBackup / 2)) ]; do
local rotation=$(($todayInDay & $i))

if [ $rotation -eq 0 ]; then
daysElapsed=$(($i * 2))
break
fi

daysElapsed=$(($daysToBackup / 2))
i=$((2 * $i))
done

echo $daysElapsed



# ----------------------------------------------------------------------------------
# Main function.
# ----------------------------------------------------------------------------------
function main()
parseArgs $@

local todayDatetime=$(date +$DATE_FORMAT.$HOUR_FORMAT)

if createBackUp $todayDatetime; then
updateLatestLinks

local daysElapsed=$(calculateDaysFromBackupOfSameSet)
local expiredDay=$(date -d "$daysElapsed days ago" +$DATE_FORMAT)
$cmdPrefix rm -frv "$dstDir/$expiredDay."*
fi


main $@






bash file-system






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Mar 15 at 19:19









Zeta

15.5k23975




15.5k23975










asked Mar 15 at 5:18









hlapointehlapointe

178217




178217











  • $begingroup$
    I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
    $endgroup$
    – Zeta
    Mar 15 at 19:19











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    Mar 15 at 19:25
















  • $begingroup$
    I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
    $endgroup$
    – Zeta
    Mar 15 at 19:19











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    Mar 15 at 19:25















$begingroup$
I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
$endgroup$
– Zeta
Mar 15 at 19:19





$begingroup$
I have rolled back the last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers.
$endgroup$
– Zeta
Mar 15 at 19:19













$begingroup$
@Zeta, understood.
$endgroup$
– hlapointe
Mar 15 at 19:25




$begingroup$
@Zeta, understood.
$endgroup$
– hlapointe
Mar 15 at 19:25










2 Answers
2






active

oldest

votes


















10












$begingroup$

Do not silently ignore invalid arguments



Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs():




-s|--src)
if [ -d "$2" ]; then
srcDir="$2"
fi
shift 2
;;


If --src <sourceDir> is specified with an invalid source directory then this argument is (silently) ignored. The consequence is that even a simple typo




backup --src /my_importatn_data


causes the default directory to be backed up, and not /my_important_data.



Wrong arguments (here: an invalid or not existing directory) should print an error message (to the standard error) and terminate the shell script (with a non-zero exit code).






share|improve this answer









$endgroup$




















    8












    $begingroup$

    Double-quote variables used as command arguments



    Although in many places in the posted code the variables used as command arguments are correctly double-quoted, there are more than a few exceptions, for example parseArgs $@, which should be parseArgs "$@" to preserve arguments with spaces.



    It's a good habit to systematically double-quote variables used as command arguments.



    Collect argument lists in arrays instead of strings



    The createBackUp function collects arguments for the rsync command in a string and executes it. This will not work if some elements (here $srcDir, $snapDir, or $LATEST_SNAP) contain spaces.



    You can make it safe by using an array instead:



    createBackUp() 
    snapDir="$dstDir/$1" # ................. Name of the snap dir.
    logFile="$logDir/$1.log" # ............. Name of the log file.

    RSYNC_CMD=("rsync") # ................... Rsync Command.
    RSYNC_CMD+=("--archive") # .............. Enable recursion and preserve infos.
    RSYNC_CMD+=("--verbose") # .............. Increase amount of infos printed.
    RSYNC_CMD+=("--human-readable") # ....... Output number in readable format.
    RSYNC_CMD+=("--progress") # ............. Show progress during transfer.
    RSYNC_CMD+=("--delete") # ............... Delete files from receiving side.
    RSYNC_CMD+=("--link-dest=$LATEST_SNAP") # "latest" symbolic link to hardlink.
    RSYNC_CMD+=("$srcDir") # ................ Source directory to backup.
    RSYNC_CMD+=("$snapDir") # ............... Destination directory to backup in.

    # Create backup and save the output in a log file.
    $cmdPrefix "$RSYNC_CMD[@]" 2>&1


    I also dropped the function keyword which is not recommended in Bash.



    Simplify using arithmetic context



    The calculateDaysFromBackupOfSameSet uses several arithmetic operations and conditions which could be simplified and made more readable using arithmetic context:



    calculateDaysFromBackupOfSameSet() 
    local i todayInDay daysToBackup
    local todayInSec=$(date +%s)

    ((todayInDay = todayInSec / SEC_PER_DAY))
    ((daysToBackup = 2 ** (setsMax - 2)))
    local daysElapsed=daysToBackup

    for ((i = 1; i < daysToBackup; i *= 2)); do
    if ! ((todayInDay & i)); then
    ((daysElapsed = i * 2))
    break
    fi
    done

    echo "$daysElapsed"



    I also eliminated some repeated daysToBackup / 2.
    I believe this is equivalent to the original, but I haven't tested it.
    Excuse me if I made an error, the point is more to demonstrate the power of the arithmetic context.






    share|improve this answer











    $endgroup$













      Your Answer






      StackExchange.ifUsing("editor", function ()
      StackExchange.using("externalEditor", function ()
      StackExchange.using("snippets", function ()
      StackExchange.snippets.init();
      );
      );
      , "code-snippets");

      StackExchange.ready(function()
      var channelOptions =
      tags: "".split(" "),
      id: "196"
      ;
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function()
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled)
      StackExchange.using("snippets", function()
      createEditor();
      );

      else
      createEditor();

      );

      function createEditor()
      StackExchange.prepareEditor(
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader:
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      ,
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      );



      );













      draft saved

      draft discarded


















      StackExchange.ready(
      function ()
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215473%2fbackup-with-hanoi-strategy%23new-answer', 'question_page');

      );

      Post as a guest















      Required, but never shown

























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      10












      $begingroup$

      Do not silently ignore invalid arguments



      Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs():




      -s|--src)
      if [ -d "$2" ]; then
      srcDir="$2"
      fi
      shift 2
      ;;


      If --src <sourceDir> is specified with an invalid source directory then this argument is (silently) ignored. The consequence is that even a simple typo




      backup --src /my_importatn_data


      causes the default directory to be backed up, and not /my_important_data.



      Wrong arguments (here: an invalid or not existing directory) should print an error message (to the standard error) and terminate the shell script (with a non-zero exit code).






      share|improve this answer









      $endgroup$

















        10












        $begingroup$

        Do not silently ignore invalid arguments



        Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs():




        -s|--src)
        if [ -d "$2" ]; then
        srcDir="$2"
        fi
        shift 2
        ;;


        If --src <sourceDir> is specified with an invalid source directory then this argument is (silently) ignored. The consequence is that even a simple typo




        backup --src /my_importatn_data


        causes the default directory to be backed up, and not /my_important_data.



        Wrong arguments (here: an invalid or not existing directory) should print an error message (to the standard error) and terminate the shell script (with a non-zero exit code).






        share|improve this answer









        $endgroup$















          10












          10








          10





          $begingroup$

          Do not silently ignore invalid arguments



          Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs():




          -s|--src)
          if [ -d "$2" ]; then
          srcDir="$2"
          fi
          shift 2
          ;;


          If --src <sourceDir> is specified with an invalid source directory then this argument is (silently) ignored. The consequence is that even a simple typo




          backup --src /my_importatn_data


          causes the default directory to be backed up, and not /my_important_data.



          Wrong arguments (here: an invalid or not existing directory) should print an error message (to the standard error) and terminate the shell script (with a non-zero exit code).






          share|improve this answer









          $endgroup$



          Do not silently ignore invalid arguments



          Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs():




          -s|--src)
          if [ -d "$2" ]; then
          srcDir="$2"
          fi
          shift 2
          ;;


          If --src <sourceDir> is specified with an invalid source directory then this argument is (silently) ignored. The consequence is that even a simple typo




          backup --src /my_importatn_data


          causes the default directory to be backed up, and not /my_important_data.



          Wrong arguments (here: an invalid or not existing directory) should print an error message (to the standard error) and terminate the shell script (with a non-zero exit code).







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Mar 15 at 6:21









          Martin RMartin R

          16.3k12366




          16.3k12366























              8












              $begingroup$

              Double-quote variables used as command arguments



              Although in many places in the posted code the variables used as command arguments are correctly double-quoted, there are more than a few exceptions, for example parseArgs $@, which should be parseArgs "$@" to preserve arguments with spaces.



              It's a good habit to systematically double-quote variables used as command arguments.



              Collect argument lists in arrays instead of strings



              The createBackUp function collects arguments for the rsync command in a string and executes it. This will not work if some elements (here $srcDir, $snapDir, or $LATEST_SNAP) contain spaces.



              You can make it safe by using an array instead:



              createBackUp() 
              snapDir="$dstDir/$1" # ................. Name of the snap dir.
              logFile="$logDir/$1.log" # ............. Name of the log file.

              RSYNC_CMD=("rsync") # ................... Rsync Command.
              RSYNC_CMD+=("--archive") # .............. Enable recursion and preserve infos.
              RSYNC_CMD+=("--verbose") # .............. Increase amount of infos printed.
              RSYNC_CMD+=("--human-readable") # ....... Output number in readable format.
              RSYNC_CMD+=("--progress") # ............. Show progress during transfer.
              RSYNC_CMD+=("--delete") # ............... Delete files from receiving side.
              RSYNC_CMD+=("--link-dest=$LATEST_SNAP") # "latest" symbolic link to hardlink.
              RSYNC_CMD+=("$srcDir") # ................ Source directory to backup.
              RSYNC_CMD+=("$snapDir") # ............... Destination directory to backup in.

              # Create backup and save the output in a log file.
              $cmdPrefix "$RSYNC_CMD[@]" 2>&1


              I also dropped the function keyword which is not recommended in Bash.



              Simplify using arithmetic context



              The calculateDaysFromBackupOfSameSet uses several arithmetic operations and conditions which could be simplified and made more readable using arithmetic context:



              calculateDaysFromBackupOfSameSet() 
              local i todayInDay daysToBackup
              local todayInSec=$(date +%s)

              ((todayInDay = todayInSec / SEC_PER_DAY))
              ((daysToBackup = 2 ** (setsMax - 2)))
              local daysElapsed=daysToBackup

              for ((i = 1; i < daysToBackup; i *= 2)); do
              if ! ((todayInDay & i)); then
              ((daysElapsed = i * 2))
              break
              fi
              done

              echo "$daysElapsed"



              I also eliminated some repeated daysToBackup / 2.
              I believe this is equivalent to the original, but I haven't tested it.
              Excuse me if I made an error, the point is more to demonstrate the power of the arithmetic context.






              share|improve this answer











              $endgroup$

















                8












                $begingroup$

                Double-quote variables used as command arguments



                Although in many places in the posted code the variables used as command arguments are correctly double-quoted, there are more than a few exceptions, for example parseArgs $@, which should be parseArgs "$@" to preserve arguments with spaces.



                It's a good habit to systematically double-quote variables used as command arguments.



                Collect argument lists in arrays instead of strings



                The createBackUp function collects arguments for the rsync command in a string and executes it. This will not work if some elements (here $srcDir, $snapDir, or $LATEST_SNAP) contain spaces.



                You can make it safe by using an array instead:



                createBackUp() 
                snapDir="$dstDir/$1" # ................. Name of the snap dir.
                logFile="$logDir/$1.log" # ............. Name of the log file.

                RSYNC_CMD=("rsync") # ................... Rsync Command.
                RSYNC_CMD+=("--archive") # .............. Enable recursion and preserve infos.
                RSYNC_CMD+=("--verbose") # .............. Increase amount of infos printed.
                RSYNC_CMD+=("--human-readable") # ....... Output number in readable format.
                RSYNC_CMD+=("--progress") # ............. Show progress during transfer.
                RSYNC_CMD+=("--delete") # ............... Delete files from receiving side.
                RSYNC_CMD+=("--link-dest=$LATEST_SNAP") # "latest" symbolic link to hardlink.
                RSYNC_CMD+=("$srcDir") # ................ Source directory to backup.
                RSYNC_CMD+=("$snapDir") # ............... Destination directory to backup in.

                # Create backup and save the output in a log file.
                $cmdPrefix "$RSYNC_CMD[@]" 2>&1


                I also dropped the function keyword which is not recommended in Bash.



                Simplify using arithmetic context



                The calculateDaysFromBackupOfSameSet uses several arithmetic operations and conditions which could be simplified and made more readable using arithmetic context:



                calculateDaysFromBackupOfSameSet() 
                local i todayInDay daysToBackup
                local todayInSec=$(date +%s)

                ((todayInDay = todayInSec / SEC_PER_DAY))
                ((daysToBackup = 2 ** (setsMax - 2)))
                local daysElapsed=daysToBackup

                for ((i = 1; i < daysToBackup; i *= 2)); do
                if ! ((todayInDay & i)); then
                ((daysElapsed = i * 2))
                break
                fi
                done

                echo "$daysElapsed"



                I also eliminated some repeated daysToBackup / 2.
                I believe this is equivalent to the original, but I haven't tested it.
                Excuse me if I made an error, the point is more to demonstrate the power of the arithmetic context.






                share|improve this answer











                $endgroup$















                  8












                  8








                  8





                  $begingroup$

                  Double-quote variables used as command arguments



                  Although in many places in the posted code the variables used as command arguments are correctly double-quoted, there are more than a few exceptions, for example parseArgs $@, which should be parseArgs "$@" to preserve arguments with spaces.



                  It's a good habit to systematically double-quote variables used as command arguments.



                  Collect argument lists in arrays instead of strings



                  The createBackUp function collects arguments for the rsync command in a string and executes it. This will not work if some elements (here $srcDir, $snapDir, or $LATEST_SNAP) contain spaces.



                  You can make it safe by using an array instead:



                  createBackUp() 
                  snapDir="$dstDir/$1" # ................. Name of the snap dir.
                  logFile="$logDir/$1.log" # ............. Name of the log file.

                  RSYNC_CMD=("rsync") # ................... Rsync Command.
                  RSYNC_CMD+=("--archive") # .............. Enable recursion and preserve infos.
                  RSYNC_CMD+=("--verbose") # .............. Increase amount of infos printed.
                  RSYNC_CMD+=("--human-readable") # ....... Output number in readable format.
                  RSYNC_CMD+=("--progress") # ............. Show progress during transfer.
                  RSYNC_CMD+=("--delete") # ............... Delete files from receiving side.
                  RSYNC_CMD+=("--link-dest=$LATEST_SNAP") # "latest" symbolic link to hardlink.
                  RSYNC_CMD+=("$srcDir") # ................ Source directory to backup.
                  RSYNC_CMD+=("$snapDir") # ............... Destination directory to backup in.

                  # Create backup and save the output in a log file.
                  $cmdPrefix "$RSYNC_CMD[@]" 2>&1


                  I also dropped the function keyword which is not recommended in Bash.



                  Simplify using arithmetic context



                  The calculateDaysFromBackupOfSameSet uses several arithmetic operations and conditions which could be simplified and made more readable using arithmetic context:



                  calculateDaysFromBackupOfSameSet() 
                  local i todayInDay daysToBackup
                  local todayInSec=$(date +%s)

                  ((todayInDay = todayInSec / SEC_PER_DAY))
                  ((daysToBackup = 2 ** (setsMax - 2)))
                  local daysElapsed=daysToBackup

                  for ((i = 1; i < daysToBackup; i *= 2)); do
                  if ! ((todayInDay & i)); then
                  ((daysElapsed = i * 2))
                  break
                  fi
                  done

                  echo "$daysElapsed"



                  I also eliminated some repeated daysToBackup / 2.
                  I believe this is equivalent to the original, but I haven't tested it.
                  Excuse me if I made an error, the point is more to demonstrate the power of the arithmetic context.






                  share|improve this answer











                  $endgroup$



                  Double-quote variables used as command arguments



                  Although in many places in the posted code the variables used as command arguments are correctly double-quoted, there are more than a few exceptions, for example parseArgs $@, which should be parseArgs "$@" to preserve arguments with spaces.



                  It's a good habit to systematically double-quote variables used as command arguments.



                  Collect argument lists in arrays instead of strings



                  The createBackUp function collects arguments for the rsync command in a string and executes it. This will not work if some elements (here $srcDir, $snapDir, or $LATEST_SNAP) contain spaces.



                  You can make it safe by using an array instead:



                  createBackUp() 
                  snapDir="$dstDir/$1" # ................. Name of the snap dir.
                  logFile="$logDir/$1.log" # ............. Name of the log file.

                  RSYNC_CMD=("rsync") # ................... Rsync Command.
                  RSYNC_CMD+=("--archive") # .............. Enable recursion and preserve infos.
                  RSYNC_CMD+=("--verbose") # .............. Increase amount of infos printed.
                  RSYNC_CMD+=("--human-readable") # ....... Output number in readable format.
                  RSYNC_CMD+=("--progress") # ............. Show progress during transfer.
                  RSYNC_CMD+=("--delete") # ............... Delete files from receiving side.
                  RSYNC_CMD+=("--link-dest=$LATEST_SNAP") # "latest" symbolic link to hardlink.
                  RSYNC_CMD+=("$srcDir") # ................ Source directory to backup.
                  RSYNC_CMD+=("$snapDir") # ............... Destination directory to backup in.

                  # Create backup and save the output in a log file.
                  $cmdPrefix "$RSYNC_CMD[@]" 2>&1


                  I also dropped the function keyword which is not recommended in Bash.



                  Simplify using arithmetic context



                  The calculateDaysFromBackupOfSameSet uses several arithmetic operations and conditions which could be simplified and made more readable using arithmetic context:



                  calculateDaysFromBackupOfSameSet() 
                  local i todayInDay daysToBackup
                  local todayInSec=$(date +%s)

                  ((todayInDay = todayInSec / SEC_PER_DAY))
                  ((daysToBackup = 2 ** (setsMax - 2)))
                  local daysElapsed=daysToBackup

                  for ((i = 1; i < daysToBackup; i *= 2)); do
                  if ! ((todayInDay & i)); then
                  ((daysElapsed = i * 2))
                  break
                  fi
                  done

                  echo "$daysElapsed"



                  I also eliminated some repeated daysToBackup / 2.
                  I believe this is equivalent to the original, but I haven't tested it.
                  Excuse me if I made an error, the point is more to demonstrate the power of the arithmetic context.







                  share|improve this answer














                  share|improve this answer



                  share|improve this answer








                  edited Mar 16 at 5:43

























                  answered Mar 15 at 7:34









                  janosjanos

                  99.6k12127351




                  99.6k12127351



























                      draft saved

                      draft discarded
















































                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid


                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.

                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function ()
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215473%2fbackup-with-hanoi-strategy%23new-answer', 'question_page');

                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown






                      Popular posts from this blog

                      Peggy Mitchell

                      Palaiologos

                      The Forum (Inglewood, California)