Backup with Hanoi StrategySimple interactive backup scriptMinecraft auto-backup utilityBash Backup ScriptPush backup scriptRegular backup/snapshotsSimple Bash log backup scriptBash “Rotating” MySQL backupFiletype backup scriptSelective Time Machine backup deletionBackup a SQLite database

How difficult is it to simply disable/disengage the MCAS on Boeing 737 Max 8 & 9 Aircraft?

Knife as defense against stray dogs

Why did it take so long to abandon sail after steamships were demonstrated?

What is a ^ b and (a & b) << 1?

Brexit - No Deal Rejection

Shortcut for setting origin to vertice

How to make healing in an exploration game interesting

Why is the President allowed to veto a cancellation of emergency powers?

Is it normal that my co-workers at a fitness company criticize my food choices?

Do I need life insurance if I can cover my own funeral costs?

Convergence in probability and convergence in distribution

What is the adequate fee for a reveal operation?

Violin - Can double stops be played when the strings are not next to each other?

Unexpected result from ArcLength

Why does energy conservation give me the wrong answer in this inelastic collision problem?

How to deal with taxi scam when on vacation?

Is honey really a supersaturated solution? Does heating to un-crystalize redissolve it or melt it?

Russian cases: A few examples, I'm really confused

Bacteria contamination inside a thermos bottle

Did Ender ever learn that he killed Stilson and/or Bonzo?

How do I change two letters closest to a string and one letter immediately after a string using Notepad++?

Pauli exclusion principle

What is the significance behind "40 days" that often appears in the Bible?

My adviser wants to be the first author



Backup with Hanoi Strategy


Simple interactive backup scriptMinecraft auto-backup utilityBash Backup ScriptPush backup scriptRegular backup/snapshotsSimple Bash log backup scriptBash “Rotating” MySQL backupFiletype backup scriptSelective Time Machine backup deletionBackup a SQLite database













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() --dst)
if [ -d "$2" ]; then
dstDir="$2"
fi
shift 2
;;

-l


# ----------------------------------------------------------------------------------
# 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
    22 hours ago











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    22 hours ago















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() --dst)
if [ -d "$2" ]; then
dstDir="$2"
fi
shift 2
;;

-l


# ----------------------------------------------------------------------------------
# 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
    22 hours ago











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    22 hours ago













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() --dst)
if [ -d "$2" ]; then
dstDir="$2"
fi
shift 2
;;

-l


# ----------------------------------------------------------------------------------
# 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() --dst)
if [ -d "$2" ]; then
dstDir="$2"
fi
shift 2
;;

-l


# ----------------------------------------------------------------------------------
# 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 22 hours ago









Zeta

15.3k23875




15.3k23875










asked yesterday









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
    22 hours ago











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    22 hours ago
















  • $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
    22 hours ago











  • $begingroup$
    @Zeta, understood.
    $endgroup$
    – hlapointe
    22 hours ago















$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
22 hours ago





$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
22 hours ago













$begingroup$
@Zeta, understood.
$endgroup$
– hlapointe
22 hours ago




$begingroup$
@Zeta, understood.
$endgroup$
– hlapointe
22 hours ago










2 Answers
2






active

oldest

votes


















9












$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$




















    6












    $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$












    • $begingroup$
      I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
      $endgroup$
      – hlapointe
      23 hours ago










    • $begingroup$
      Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
      $endgroup$
      – hlapointe
      23 hours ago










    • $begingroup$
      Is it good practice to also double-quote argument passed to a function even if the argument is integer?
      $endgroup$
      – hlapointe
      22 hours ago










    • $begingroup$
      The logic seem to work! :P
      $endgroup$
      – hlapointe
      20 hours ago










    • $begingroup$
      @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
      $endgroup$
      – janos
      12 hours ago










    Your Answer





    StackExchange.ifUsing("editor", function ()
    return StackExchange.using("mathjaxEditing", function ()
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    );
    );
    , "mathjax-editing");

    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









    9












    $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$

















      9












      $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$















        9












        9








        9





        $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 yesterday









        Martin RMartin R

        16.2k12365




        16.2k12365























            6












            $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$












            • $begingroup$
              I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Is it good practice to also double-quote argument passed to a function even if the argument is integer?
              $endgroup$
              – hlapointe
              22 hours ago










            • $begingroup$
              The logic seem to work! :P
              $endgroup$
              – hlapointe
              20 hours ago










            • $begingroup$
              @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
              $endgroup$
              – janos
              12 hours ago















            6












            $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$












            • $begingroup$
              I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Is it good practice to also double-quote argument passed to a function even if the argument is integer?
              $endgroup$
              – hlapointe
              22 hours ago










            • $begingroup$
              The logic seem to work! :P
              $endgroup$
              – hlapointe
              20 hours ago










            • $begingroup$
              @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
              $endgroup$
              – janos
              12 hours ago













            6












            6








            6





            $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 12 hours ago

























            answered yesterday









            janosjanos

            98.6k12125350




            98.6k12125350











            • $begingroup$
              I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Is it good practice to also double-quote argument passed to a function even if the argument is integer?
              $endgroup$
              – hlapointe
              22 hours ago










            • $begingroup$
              The logic seem to work! :P
              $endgroup$
              – hlapointe
              20 hours ago










            • $begingroup$
              @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
              $endgroup$
              – janos
              12 hours ago
















            • $begingroup$
              I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
              $endgroup$
              – hlapointe
              23 hours ago










            • $begingroup$
              Is it good practice to also double-quote argument passed to a function even if the argument is integer?
              $endgroup$
              – hlapointe
              22 hours ago










            • $begingroup$
              The logic seem to work! :P
              $endgroup$
              – hlapointe
              20 hours ago










            • $begingroup$
              @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
              $endgroup$
              – janos
              12 hours ago















            $begingroup$
            I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
            $endgroup$
            – hlapointe
            23 hours ago




            $begingroup$
            I also saw that you did reverse the iteration of the loop, is it only to be more readable or there's a performance avantage?
            $endgroup$
            – hlapointe
            23 hours ago












            $begingroup$
            Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
            $endgroup$
            – hlapointe
            23 hours ago




            $begingroup$
            Why change daysToBackup = 2 ** (setsMax - 1) to daysToBackup = 2 ** (setsMax - 2).
            $endgroup$
            – hlapointe
            23 hours ago












            $begingroup$
            Is it good practice to also double-quote argument passed to a function even if the argument is integer?
            $endgroup$
            – hlapointe
            22 hours ago




            $begingroup$
            Is it good practice to also double-quote argument passed to a function even if the argument is integer?
            $endgroup$
            – hlapointe
            22 hours ago












            $begingroup$
            The logic seem to work! :P
            $endgroup$
            – hlapointe
            20 hours ago




            $begingroup$
            The logic seem to work! :P
            $endgroup$
            – hlapointe
            20 hours ago












            $begingroup$
            @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
            $endgroup$
            – janos
            12 hours ago




            $begingroup$
            @hlapointe I don't know what you mean by "reverse the iteration of the loop". I replaced daysToBackup / 2 with daysToBackup, and accordingly daysToBackup = 2 ** (setsMax - 1) with daysToBackup = 2 ** (setsMax - 2). And yes it's a good practice to systematically double-quote even values you know don't contain anything unsafe.
            $endgroup$
            – janos
            12 hours ago

















            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

            Is flight data recorder erased after every flight?When are black boxes used?What protects the location beacon (pinger) of a flight data recorder?Is there anywhere I can pick up raw flight data recorder information?Who legally owns the Flight Data Recorder?Constructing flight recorder dataWhy are FDRs and CVRs still two separate physical devices?What are the data elements shown on the GE235 flight data recorder (FDR) plot?Are CVR and FDR reset after every flight?What is the format of data stored by a Flight Data Recorder?How much data is stored in the flight data recorder per hour in a typical flight of an A380?Is a smart flight data recorder possible?

            Which is better: GPT or RelGAN for text generation?2019 Community Moderator ElectionWhat is the difference between TextGAN and LM for text generation?GANs (generative adversarial networks) possible for text as well?Generator loss not decreasing- text to image synthesisChoosing a right algorithm for template-based text generationHow should I format input and output for text generation with LSTMsGumbel Softmax vs Vanilla Softmax for GAN trainingWhich neural network to choose for classification from text/speech?NLP text autoencoder that generates text in poetic meterWhat is the interpretation of the expectation notation in the GAN formulation?What is the difference between TextGAN and LM for text generation?How to prepare the data for text generation task

            Is there a general name for the setup in which payoffs are not known exactly but players try to influence each other's perception of the payoffs?Osborne, Nash equilibria and the correctness of beliefsIs there a name for this family of games (Binomial games?)?Perfect Bayesian EquilibriumCalculating mixed strategy equilibrium in battle of sexesPure Strategy SPNEIs there a commitment mechanism which allows players to achieve pareto optimal solutions?Extensive Form GamesAn $n$-player prisoner's dilemma where a coalition of 2 players is better off defectingTit-For-Stat Strategy Best RepliesPotential solutions of the $n$-player Prisoner's Dilemma