TT#82051 Shellcheck fixes

shellcheck v0.7.1 complains about a bunch of issues: SC1083: This { is literal. Check expression (missing ;/\n?) or quote it. SC1083: This } is literal. Check expression (missing ;/\n?) or quote it. SC1090: Can't follow non-constant source. Use a directive to specify location. SC2034: foo appears unused. Verify it or export it. SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2128: Expanding an array without an index only gives the first element. SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive. SC2155: Declare and assign separately to avoid masking return values. SC2162: read without -r will mangle backslashes. SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?. SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting). SC2237: Use [ -z .. ] instead of ! [ -n .. ]. We're still bound to support shellcheck v0.3.4 and sadly this version fails hard when using the modern "shellcheck source=" directives to address SC1090 and SC1091. It's failing like: | not ok 1 source/functions/init:8:28: note: The mentioned parser error was in this then clause. [SC1009] | not ok 2 source/functions/init:10:3: error: Couldn't parse this shellcheck annotation. [SC1073] | not ok 3 source/functions/init:10:16: error: Unexpected "s". Fix any mentioned problems and try again. [SC1072] So until we can get rid of supporting this ancient shellcheck version, instead of using: | # shellcheck source=etc/ngcp-config/ngcpcfg.cfg | # shellcheck source=functions/init | # shellcheck source=functions/logs | # shellcheck source=functions/main we need to place disable=SC1090 and disable=SC1091 accordingly. Change-Id: I14da87a00102609bffdb60e932045180c6d13d5e
5 years ago · 0dda93ad97
parent d5863ed74b
commit 0dda93ad97
23 changed files with 76 additions and 34 deletions
--- a/etc/ngcp-config/ngcpcfg.cfg
+++ b/etc/ngcp-config/ngcpcfg.cfg
@ -2,6 +2,10 @@
 # Purpose:  main configuration file for ngcpcfg tools
 # Note:     do not modify unless you have a really good reason to do so

+# This directive applies to the entire script
+# shellcheck shell=bash
+# shellcheck disable=SC2034
+
 # directory name where ngcpcfg is managed through git
 NGCPCTL_BASE='/etc'
 NGCPCTL_MAIN="${NGCPCTL_BASE}/ngcp-config"
--- a/functions/init
+++ b/functions/init
@ -7,15 +7,18 @@
 # support test suite which requires system independent configuration
 if [ -r "${NGCPCFG:-}" ] ; then
  log_debug "sourcing configuration file ${NGCPCFG:-}"
+  # shellcheck disable=SC1090
  . "${NGCPCFG:-}"
 else
  if [ -r /etc/ngcp-config/ngcpcfg.cfg ] ; then
+    # shellcheck disable=SC1091
    . /etc/ngcp-config/ngcpcfg.cfg
    log_debug "sourced configuration file /etc/ngcp-config/ngcpcfg.cfg"

    if [ -d /etc/ngcp-config/ngcpcfg.d ] ; then
      for file in /etc/ngcp-config/ngcpcfg.d/*.cfg ; do
        if [ -r "${file}" ]; then
+          # shellcheck disable=SC1090
          . "${file}"
          log_debug "sourced configuration file '${file}'"
        fi
@ -40,7 +43,7 @@ if ! [ -r "${CONSTANTS_CONFIG}" ] ; then
  exit 1
 fi

-if ! [ -n "${NETWORK_CONFIG:-}" ] ; then
+if [ -z "${NETWORK_CONFIG:-}" ] ; then
  log_warn "NETWORK_CONFIG is not configured in ${NGCPCTL_CONFIG} - continuing anyway."
 elif ! [ -r "${NETWORK_CONFIG}" ] ; then
  log_error "Constants file ${NETWORK_CONFIG} does not exist (unconfigured?) - exiting."
@ -61,8 +64,10 @@ fi

 ## environment variables {{{
 export PN="ngcpcfg"
-export HNAME="$(hostname)"
-export NNAME="$(ngcp-nodename)"
+export HNAME
+HNAME="$(hostname)"
+export NNAME
+NNAME="$(ngcp-nodename)"

 # avoid warnings by perl script complaining about locales
 export LANG=C
--- a/functions/logs
+++ b/functions/logs
@ -8,15 +8,17 @@ export timestamp_replacementchars='' # unset by default

 console_output() {
  if [ -z "${TIME_FORMAT:-}" ] ; then
-    printf -- "$*"
+    printf -- "%s" "$*"
    return 0
  fi

-  local timestamp="$(date "$TIME_FORMAT")"
+  local timestamp
+  timestamp="$(date "$TIME_FORMAT")"

  # indent depending on number of characters in date output
-  export timestamp_replacementchars="$(printf -- "%s: " "$timestamp" | sed 's/./ /g')"
-  printf -- "$timestamp ${HNAME:-}: $*"
+  export timestamp_replacementchars
+  timestamp_replacementchars="$(printf -- "%s: " "$timestamp" | sed 's/./ /g')"
+  printf -- "%s" "$timestamp ${HNAME:-}: $*"
 }
 # }}}

--- a/functions/main
+++ b/functions/main
@ -1,6 +1,7 @@
 # Filename:      /usr/share/ngcp-ngcpcfg/functions/main
 # Purpose:       helper functions for ngcpcfg
 ################################################################################
+# shellcheck shell=bash

 # support for testsuite, assume defaults if unset
 FUNCTIONS="${FUNCTIONS:-/usr/share/ngcp-ngcpcfg/functions/}"
@ -9,16 +10,19 @@ if ! [ -r "${FUNCTIONS}"/logs ] ; then
  printf "Error: %s/logs could not be read. Exiting.\n" "${FUNCTIONS}" >&2
  exit 1
 fi
+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/logs

 ## HA / carrier features {{{
 if [ -r "${FUNCTIONS}"/ha_features ] ; then
+  # shellcheck disable=SC1090
  . "${FUNCTIONS}"/ha_features
  # Set ${HA_FILE}, used by ngcpcfg.cfg, tt2-process and encrypt.
  set_ha_file
 fi

 if [ -r "${FUNCTIONS}"/carrier_features ] ; then
+  # shellcheck disable=SC1090
  . "${FUNCTIONS}"/carrier_features
  # Set ${HOST_FILE} + ${PAIR_FILE}, used by ngcpcfg.cfg and tt2-process.
  set_host_and_pair_files
@ -30,6 +34,7 @@ if ! [ -r "${FUNCTIONS}"/init ] ; then
  exit 1
 fi
 set -o allexport
+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/init
 set +o allexport

@ -97,7 +102,7 @@ compare_active_branch() {
  log_debug "get_active_branch ${1:-}"

  log_debug "cd ${NGCPCTL_MAIN}"
-  cd "${NGCPCTL_MAIN}"
+  cd "${NGCPCTL_MAIN}" || return

  local current_branch
  current_branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)
@ -107,16 +112,19 @@ compare_active_branch() {

 get_branch_status() {
  log_debug "cd ${NGCPCTL_MAIN}"
-  cd "${NGCPCTL_MAIN}"
+  cd "${NGCPCTL_MAIN}" || return

  log_debug "git rev-parse HEAD"
-  local LOCAL=$(git rev-parse HEAD)
+  local LOCAL
+  LOCAL=$(git rev-parse HEAD)

  log_debug "git rev-parse @{u}"
-  local REMOTE=$(git rev-parse @{u})
+  local REMOTE
+  REMOTE=$(git rev-parse '@{u}')

  log_debug "git merge-base HEAD @{u}"
-  local BASE=$(git merge-base HEAD @{u})
+  local BASE
+  BASE=$(git merge-base HEAD '@{u}')

  if [ "${LOCAL}" = "${REMOTE}" ]; then
    # Up-to-date
@ -135,7 +143,7 @@ get_branch_status() {

 record_commit_id() {
  log_debug "cd ${NGCPCTL_MAIN}"
-  cd "${NGCPCTL_MAIN}"
+  cd "${NGCPCTL_MAIN}" || return

  log_debug "mkdir -p ${STATE_FILES_DIR}"
  mkdir -p "${STATE_FILES_DIR}"
@ -144,7 +152,8 @@ record_commit_id() {
  if git status --porcelain | grep -q . ; then
    echo "dirty" > "${STATE_FILES_DIR}/build"
  else
-    local latest_commit=$(git log -1 --format="%H")
+    local latest_commit
+    latest_commit=$(git log -1 --format="%H")
    log_debug "echo ${latest_commit} > ${STATE_FILES_DIR}/build"
    echo "${latest_commit}" > "${STATE_FILES_DIR}/build"
  fi
--- a/sbin/ngcpcfg
+++ b/sbin/ngcpcfg
@ -131,6 +131,7 @@ if [[ "${1:-}" == "decrypt" ]] ; then
    fi
  }
 else
+  # shellcheck disable=SC1090
  . "${FUNCTIONS}"/main
 fi

--- a/scripts/apply
+++ b/scripts/apply
@ -16,6 +16,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # based on check_local_state() from scripts/status
--- a/scripts/build
+++ b/scripts/build
@ -16,6 +16,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 MODIFIED_ONLY=false
--- a/scripts/check
+++ b/scripts/check
@ -14,6 +14,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 usage() {
@ -40,7 +41,8 @@ check_branch() {
    return 0
  fi

-  local current_branch=$(compare_active_branch 'master')
+  local current_branch
+  current_branch=$(compare_active_branch 'master')
  if [ "$current_branch" = 'master' ] ; then
    log_debug "Current branch is 'master', continuing."
  else
@ -66,6 +68,7 @@ check_origin() {
  cd "${NGCPCTL_MAIN}"

  if [ -r /etc/default/ngcp-roles ]; then
+    # shellcheck disable=SC1091
    . /etc/default/ngcp-roles
  else
    NGCP_IS_MGMT="yes"
--- a/scripts/clean
+++ b/scripts/clean
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 cd "${NGCPCTL_MAIN}"
@ -40,7 +41,7 @@ request_confirmation() {
  while read -r -t 0; do read -r; done

  while true ; do
-    read a
+    read -r a
    case "${a,,}" in
      yes) return 0 ;;
      no) return 1 ;;
@ -294,8 +295,7 @@ tracked_files=false
 untracked_files=false

 clean_options="all,branches,force,help,reset-master,stashes,tracked-files,untracked-files"
-_opt_temp=$(getopt --name "$0" -o +h --long ${clean_options} -- "$@")
-if [ $? -ne 0 ]; then
+if ! _opt_temp=$(getopt --name "$0" -o +h --long ${clean_options} -- "$@") ; then
  log_error "Try 'ngcpcfg clean --help' for more information."
  exit 1
 fi
--- a/scripts/commit
+++ b/scripts/commit
@ -18,6 +18,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
 fi

 timestamp_replacementchars=''
+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # main script
--- a/scripts/decrypt
+++ b/scripts/decrypt
@ -20,7 +20,7 @@ timestamp_replacementchars='' # unset by default

 console_output() {
  if [ -z "${TIME_FORMAT:-}" ] ; then
-    printf -- "$*"
+    printf -- "%s" "$*"
    return 0
  fi

@ -29,7 +29,7 @@ console_output() {

  # indent depending on number of characters in date output
  timestamp_replacementchars="$(printf -- "%s: " "$timestamp" | sed 's/./ /g')"
-  printf -- "$timestamp: $*"
+  printf -- "%s" "$timestamp: $*"
 }

 log_info() {
--- a/scripts/del
+++ b/scripts/del
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 ## functions {{{
--- a/scripts/diff
+++ b/scripts/diff
@ -17,7 +17,8 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
 fi

 timestamp_replacementchars=''
-.  "${FUNCTIONS}"/main
+# shellcheck disable=SC1090
+. "${FUNCTIONS}"/main

 # main script
 log_debug "cd $NGCPCTL_MAIN"
--- a/scripts/encrypt
+++ b/scripts/encrypt
@ -14,6 +14,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 get_config_file_list() {
@ -62,7 +63,7 @@ else
 fi

 log_info_n "Now really erase all configuration files managed by ngcpcfg? [y/N] "
-a='' ; read a
+a='' ; read -r a
 if [[ "$a" == "y" ]] || [[ "$a" == "Y" ]] ; then
  rm -rf "$NGCPCTL_MAIN" ; rm -f "$TARGZ"
  # shellcheck disable=SC2086
--- a/scripts/etckeeper
+++ b/scripts/etckeeper
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
 fi

 timestamp_replacementchars=''
+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # main script
--- a/scripts/get
+++ b/scripts/get
@ -36,6 +36,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # Get the list of configs in proper order to load.
--- a/scripts/initialise
+++ b/scripts/initialise
@ -14,6 +14,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # main script
--- a/scripts/log
+++ b/scripts/log
@ -14,6 +14,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 cd "$NGCPCTL_MAIN"
--- a/scripts/patch
+++ b/scripts/patch
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 cd "${NGCPCTL_MAIN}"
@ -115,15 +116,15 @@ patch_apply() {
  patch_output=$(mktemp)

  local patch_opts=()
-  patch_opts+=(--input="${patch}")
-  patch_opts+=(--prefix=/dev/null)  # do not produce .orig backup files
-  patch_opts+=(--reject-file=-)     # do not produce .rej file
+  patch_opts+=("--input=${patch}")
+  patch_opts+=("--prefix=/dev/null")  # do not produce .orig backup files
+  patch_opts+=("--reject-file=-")     # do not produce .rej file
  patch_opts+=(-F0 -N)              # disable fuzzy logic to be as safe as possible
  if "${apply}" ; then
-    patch_opts+=(--output="${customtt}")
+    patch_opts+=("--output=${customtt}")
  else
    patch_opts+=(--dry-run)
-    patch_opts+=(--output=/dev/null)
+    patch_opts+=("--output=/dev/null")
  fi

  log_debug "Generating customtt '${customtt}' from '${patch}' (apply=${apply})"
@ -221,7 +222,7 @@ patch_import_customtt() {
 }

 patch_import() {
-  declare -a files=($(patch_search "customtt" "$@"))
+  mapfile -t files < <(patch_search "customtt" "$@")

  log_debug "Validating customtt files"
  for customtt in "${files[@]}" ; do
@ -243,7 +244,7 @@ patch_import() {
 }

 patch_patch() {
-  declare -a files=($(patch_search "patchtt" "$@"))
+  mapfile -t files < <(patch_search "patchtt" "$@")

  for patch in "${files[@]}" ; do
    log_info "Validating patch '${patch}'"
@ -274,7 +275,7 @@ patch_footer() {
    fi
  else
    log_error "Some operations above finished with an error for the file(s):"
-    bad_files_unique=($(echo "${bad_files[@]}" | tr ' ' '\n' | sort -u))
+    mapfile -t bad_files_unique < <(echo "${bad_files[@]}" | tr ' ' '\n' | sort -u)
    printf '\t%s\n' "${bad_files_unique[@]}"
    RC=1
  fi
--- a/scripts/services
+++ b/scripts/services
@ -19,6 +19,7 @@ if [[ ! -r "${FUNCTIONS}"/main ]]; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # functions
--- a/scripts/set
+++ b/scripts/set
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 ## functions {{{
--- a/scripts/show
+++ b/scripts/show
@ -14,6 +14,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
  exit 1
 fi

+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 if [ "$*" = "" ] ; then
--- a/scripts/status
+++ b/scripts/status
@ -15,6 +15,7 @@ if ! [ -r "${FUNCTIONS}"/main ] ; then
 fi

 timestamp_replacementchars=''
+# shellcheck disable=SC1090
 . "${FUNCTIONS}"/main

 # main script
@ -96,7 +97,8 @@ check_etc_state() {
 check_branch() {
  log_info "Checking currently active branch:"

-  local current_branch=$(compare_active_branch 'master')
+  local current_branch
+  current_branch=$(compare_active_branch 'master')
  if [ "$current_branch" = 'master' ] ; then
    log_info "OK:   branch master active"
  else
@ -140,14 +142,15 @@ check_reboot_requests() {
 }

 check_pending_build() {
-  log_debug "$FUNCNAME"
+  log_debug "${FUNCNAME[@]}"

  log_debug "cd $NGCPCTL_MAIN"
  cd "$NGCPCTL_MAIN"

  log_info "Checking state of pending builds:"

-  local latest_commit=$(git log -1 --format="%H")
+  local latest_commit
+  latest_commit=$(git log -1 --format="%H")
  log_debug "latest_commit = $latest_commit"

  if ! [ -f "${STATE_FILES_DIR}/build" ] ; then
@ -156,7 +159,8 @@ check_pending_build() {
    return 0
  fi

-  local latest_build_commit=$(cat "${STATE_FILES_DIR}/build")
+  local latest_build_commit
+  latest_build_commit=$(cat "${STATE_FILES_DIR}/build")
  log_debug "latest_build_commit = $latest_build_commit"

  if [ -n "$latest_commit" ] && [ -n "$latest_build_commit" ] ; then