From 2cefdaa1a5f6adac7240534e558ba9196872cbfa Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 18 Oct 2019 16:24:22 +0200 Subject: [PATCH 1/2] Fix shellcheck reported issues --- ccollect | 115 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 40 deletions(-) diff --git a/ccollect b/ccollect index 4fa6da3..6dd3fc0 100755 --- a/ccollect +++ b/ccollect @@ -27,9 +27,9 @@ set -u # # Standard variables (stolen from cconf) # -__pwd="$(pwd -P)" -__mydir="${0%/*}"; __abs_mydir="$(cd "$__mydir" && pwd -P)" -__myname=${0##*/}; __abs_myname="$__abs_mydir/$__myname" +__mydir="${0%/*}" +__abs_mydir="$(cd "$__mydir" && pwd -P)" +__myname=${0##*/} # # where to find our configuration and temporary file @@ -41,7 +41,8 @@ CPREEXEC="${CDEFAULTS}/pre_exec" CPOSTEXEC="${CDEFAULTS}/post_exec" CMARKER=".ccollect-marker" -export TMP="$(mktemp "/tmp/${__myname}.XXXXXX")" +TMP="$(mktemp "/tmp/${__myname}.XXXXXX")" +export TMP CONTROL_PIPE="/tmp/${__myname}-control-pipe" VERSION="2.6" @@ -76,6 +77,7 @@ LOCKFD=4 lock_flock() { # $1 = source to backup + # shellcheck disable=SC2059 lockfile="${LOCKDIR}/$(printf "${LOCKFILE_PATTERN}" "$1")" eval "exec ${LOCKFD}> ${lockfile}" @@ -85,6 +87,7 @@ lock_flock() unlock_flock() { # $1 = source to backup + # shellcheck disable=SC2059 lockfile="${LOCKDIR}/$(printf "${LOCKFILE_PATTERN}" "$1")" eval "exec ${LOCKFD}>&-" rm -f "${lockfile}" @@ -96,6 +99,7 @@ unlock_flock() lock_mkdir() { # $1 = source to backup + # shellcheck disable=SC2059 lockfile="${LOCKDIR}/$(printf "${LOCKFILE_PATTERN}" "$1")" mkdir "${lockfile}" && return 0 || return 1 @@ -104,6 +108,7 @@ lock_mkdir() unlock_mkdir() { # $1 = source to backup + # shellcheck disable=SC2059 lockfile="${LOCKDIR}/$(printf "${LOCKFILE_PATTERN}" "$1")" rmdir "${lockfile}" @@ -112,7 +117,7 @@ unlock_mkdir() # # determine locking tool: flock or mkdir # -if $(which flock > /dev/null 2>&1) +if command -v flock > /dev/null 2>&1 then lockf="lock_flock" unlockf="unlock_flock" @@ -137,6 +142,7 @@ LOGONLYERRORS="" # catch signals # TRAPFUNC="rm -f \"${TMP}\"" +# shellcheck disable=SC2064 trap "${TRAPFUNC}" 1 2 15 # @@ -147,7 +153,7 @@ trap "${TRAPFUNC}" 1 2 15 # see: http://www.tldp.org/LDP/abs/html/intandnonint.html _is_interactive() { - [ -t 0 -o -p /dev/stdin ] + [ -t 0 ] || [ -p /dev/stdin ] } # @@ -163,26 +169,26 @@ delete_from_file() # dirs for deletion will be moved to this trash dir inside destination dir # - for fast mv operation trash="$(mktemp -d ".trash.XXXXXX")" - while read to_remove; do + while read -r to_remove; do mv "${to_remove}" "${trash}" || _exit_err "Moving ${to_remove} to ${trash} failed." set -- "$@" "${to_remove}" if [ "${suffix}" ]; then - to_remove_no_suffix="$(echo ${to_remove} | sed "s/$suffix\$//")" + to_remove_no_suffix="$(echo "${to_remove}" | sed "s/$suffix\$//")" mv "${to_remove_no_suffix}" "${trash}" || _exit_err "Moving ${to_remove_no_suffix} to ${trash} failed." set -- "$@" "${to_remove_no_suffix}" fi done < "${file}" - _techo "Removing $@ in ${trash}..." + _techo "Removing $* in ${trash}..." empty_dir=".empty-dir" mkdir "${empty_dir}" || _exit_err "Empty directory ${empty_dir} cannot be created." [ "${VVERBOSE}" ] && echo "Starting: rsync -a --delete ${empty_dir} ${trash}" # rsync needs ending slash for directory content - rsync -a --delete "${empty_dir}/" "${trash}/" || _exit_err "Removing $@ failed." + rsync -a --delete "${empty_dir}/" "${trash}/" || _exit_err "Removing $* failed." rmdir "${trash}" || _exit_err "Removing ${trash} directory failed" rmdir "${empty_dir}" || _exit_err "Removing ${empty_dir} directory failed" - _techo "Removing $@ in ${trash} finished." + _techo "Removing $* in ${trash} finished." } display_version() @@ -232,7 +238,7 @@ unlock() # stdout version _techo_stdout() { - echo "$(${DDATE}): $@" + echo "$(${DDATE}): $*" } # syslog version @@ -260,14 +266,16 @@ _techo() { if [ "${LOGLEVEL}" = "a" ] then - set -- ${name:+"[${name}]"} $@ + # name is exported before calling this function + # shellcheck disable=SC2154 + set -- ${name:+"[${name}]"} "$@" "${_techof}" "$@" fi } _techo_err() { - _techo "Error: $@" + _techo "Error: $*" } _exit_err() @@ -368,7 +376,7 @@ else LOGLEVEL="a" fi -if [ "${LOGFILE}" -o "${SYSLOG}" ] +if [ "${LOGFILE}" ] || [ "${SYSLOG}" ] then if [ "${LOGONLYERRORS}" ] then @@ -378,8 +386,7 @@ fi # check that MAX_JOBS is natural number > 0 # empty string means run all in parallel -echo "${MAX_JOBS}" | grep -q -E '^[1-9][0-9]*$|^$' -if [ "$?" -ne 0 ] +if ! echo "${MAX_JOBS}" | grep -q -E '^[1-9][0-9]*$|^$' then _exit_err "Invalid max jobs value \"${MAX_JOBS}\"" fi @@ -412,19 +419,22 @@ if [ "${USE_ALL}" = 1 ]; then ( cd "${CSOURCES}" && ls -1 > "${TMP}" ) || \ _exit_err "Listing of sources failed. Aborting." - while read tmp; do - eval export source_${no_sources}=\"${tmp}\" - no_sources=$((${no_sources}+1)) + while read -r tmp; do + eval export "source_${no_sources}=\"${tmp}\"" + no_sources=$((no_sources + 1)) done < "${TMP}" else # # Get sources from command line # while [ "$#" -ge 1 ]; do - eval arg=\"\$1\"; shift + eval "arg=\"\$1\"" + shift - eval export source_${no_sources}=\"${arg}\" - no_sources="$((${no_sources}+1))" + # arg is assigned in the eval above + # shellcheck disable=SC2154 + eval export "source_${no_sources}=\"${arg}\"" + no_sources="$((no_sources + 1))" done fi @@ -461,6 +471,7 @@ if [ "${PARALLEL}" ]; then # fd 5 is tied to control pipe eval "exec 5<>${CONTROL_PIPE}" TRAPFUNC="${TRAPFUNC}; rm -f \"${CONTROL_PIPE}\"" + # shellcheck disable=SC2064 trap "${TRAPFUNC}" 0 1 2 15 # determine how much parallel jobs to prestart @@ -483,7 +494,7 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # Get current source # eval export name=\"\$source_${source_no}\" - source_no=$((${source_no}+1)) + source_no=$((source_no + 1)) # # Start ourself, if we want parallel execution @@ -498,12 +509,12 @@ while [ "${source_no}" -lt "${no_sources}" ]; do then # run prestart child if pending { "$0" "${INTERVAL}" "${name}"; printf '\n' >&5; } & - prestart=$((${prestart} - 1)) + prestart=$((prestart - 1)) continue else # each time a child finishes we get a line from the pipe # and then launch another child - while read line + while read -r line do { "$0" "${INTERVAL}" "${name}"; printf '\n' >&5; } & # get out of loop so we can contnue with main loop @@ -557,6 +568,7 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # redefine trap to also unlock (rm lockfile) TRAPFUNC="${TRAPFUNC}; unlock \"${name}\"" + # shellcheck disable=SC2064 trap "${TRAPFUNC}" 1 2 15 # @@ -576,10 +588,10 @@ while [ "${source_no}" -lt "${no_sources}" ]; do for opt in verbose very_verbose summary exclude rsync_options \ delete_incomplete rsync_failure_codes \ mtime quiet_if_down ; do - if [ -f "${backup}/${opt}" -o -f "${backup}/no_${opt}" ]; then - eval c_$opt=\"${backup}/$opt\" + if [ -f "${backup}/${opt}" ] || [ -f "${backup}/no_${opt}" ]; then + eval "c_$opt=\"${backup}/$opt\"" else - eval c_$opt=\"${CDEFAULTS}/$opt\" + eval "c_$opt=\"${CDEFAULTS}/$opt\"" fi done @@ -599,6 +611,8 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Sort by ctime (default) or mtime (configuration option) # + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_mtime}" ] ; then TSORT="t" else @@ -642,6 +656,8 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Exclude list # + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_exclude}" ]; then set -- "$@" "--exclude-from=${c_exclude}" fi @@ -649,6 +665,8 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Output a summary # + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_summary}" ]; then set -- "$@" "--stats" fi @@ -657,6 +675,8 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # Verbosity for rsync, rm, and mkdir # VVERBOSE="" + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_very_verbose}" ]; then set -- "$@" "-vv" VVERBOSE="-v" @@ -667,8 +687,10 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Extra options for rsync provided by the user # + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_rsync_options}" ]; then - while read line; do + while read -r line; do # Trim line. ln=$(echo "${line}" | awk '{$1=$1;print;}') # Only if ln is non zero length string. @@ -687,6 +709,8 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # Check: source is up and accepting connections (before deleting old backups!) # if ! rsync "$@" "${source}" >/dev/null 2>"${TMP}" ; then + # variable is assigned using eval + # shellcheck disable=SC2154 if [ ! -f "${c_quiet_if_down}" ]; then cat "${TMP}" fi @@ -706,10 +730,13 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Check incomplete backups (needs echo to remove newlines) # + # shellcheck disable=SC2010 ls -1 | grep "${CMARKER}\$" > "${TMP}"; ret=$? if [ "$ret" -eq 0 ]; then - _techo "Incomplete backups: $(echo $(cat "${TMP}"))" + _techo "Incomplete backups: $(cat "${TMP}")" + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "${c_delete_incomplete}" ]; then delete_from_file "${TMP}" "${CMARKER}" & fi @@ -718,12 +745,15 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # # Include current time in name, not the time when we began to remove above # - export destination_name="${INTERVAL}.$(${CDATE}).$$-${source_no}" - export destination_dir="${ddir}/${destination_name}" + destination_name="${INTERVAL}.$(${CDATE}).$$-${source_no}" + export destination_name + destination_dir="${ddir}/${destination_name}" + export destination_dir # # Check: maximum number of backups is reached? # + # shellcheck disable=SC2010 count="$(ls -1 | grep -c "^${INTERVAL}\\.")" _techo "Existing backups: ${count} Total keeping backups: ${c_interval}" @@ -731,6 +761,7 @@ while [ "${source_no}" -lt "${no_sources}" ]; do if [ "${count}" -ge "${c_interval}" ]; then # Use oldest directory as new backup destination directory. # It need not to be deleted, rsync will sync its content. + # shellcheck disable=SC2010 oldest_bak=$(ls -${TSORT}1r | grep "^${INTERVAL}\\." | head -n 1 || \ _exit_err "Listing oldest backup failed") _techo "Using ${oldest_bak} for destination dir ${destination_dir}" @@ -739,14 +770,15 @@ while [ "${source_no}" -lt "${no_sources}" ]; do touch "${destination_dir}" # We have something to remove only if count > interval. - remove="$((${count} - ${c_interval}))" + remove="$((count - c_interval))" else _techo_err "Renaming oldest backup ${oldest_bak} to ${destination_dir} failed, removing it." - remove="$((${count} - ${c_interval} + 1))" + remove="$((count - c_interval + 1))" fi if [ "${remove}" -gt 0 ]; then _techo "Removing ${remove} backup(s)..." + # shellcheck disable=SC2010 ls -${TSORT}1r | grep "^${INTERVAL}\\." | head -n "${remove}" > "${TMP}" || \ _exit_err "Listing old backups failed" @@ -760,6 +792,7 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # oldest existing destination directory. # dest_dir_name=$(basename "${destination_dir}") + # shellcheck disable=SC2010 last_dir="$(ls -${TSORT}p1 | grep '/$' | grep -v "${dest_dir_name}" | head -n 1)" || \ _exit_err "Failed to list contents of ${ddir}." @@ -793,8 +826,10 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # Check if rsync exit code indicates failure. # fail="" + # variable is assigned using eval + # shellcheck disable=SC2154 if [ -f "$c_rsync_failure_codes" ]; then - while read code ; do + while read -r code ; do if [ "$ret" = "$code" ]; then fail=1 fi @@ -831,10 +866,10 @@ while [ "${source_no}" -lt "${no_sources}" ]; do # Time calculation # end_s="$(${SDATE})" - full_seconds="$((${end_s} - ${begin_s}))" - hours="$((${full_seconds} / 3600))" - minutes="$(((${full_seconds} % 3600) / 60))" - seconds="$((${full_seconds} % 60))" + full_seconds="$((end_s - begin_s))" + hours="$((full_seconds / 3600))" + minutes="$(((full_seconds % 3600) / 60))" + seconds="$((full_seconds % 60))" _techo "Backup lasted: ${hours}:${minutes}:${seconds} (h:m:s)" From a5e565b5d69106c11c8090825038b8af51c6fdb5 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Thu, 14 Nov 2019 19:16:11 +0100 Subject: [PATCH 2/2] Add shellcheck makefile target --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index dc98502..9b07e09 100644 --- a/Makefile +++ b/Makefile @@ -199,6 +199,9 @@ dist: distclean documentation /tmp/ccollect: mkdir -p /tmp/ccollect +shellcheck: ./ccollect + shellcheck -s sh -f gcc -x ./ccollect + test: $(CCOLLECT_SOURCE) /tmp/ccollect cd ./conf/sources/; for s in *; do CCOLLECT_CONF=../ ../../ccollect daily "$$s"; done touch /tmp/ccollect/$$(ls /tmp/ccollect | head -n1).ccollect-marker