Code cleanup

Make sure local variables are defined as local before they get assigned.
Having variables get defined and assigned in one line like this:

local var=foo

can sometimes cause problems. For example, this line:

local var=$(false)

will return 0 and the script will not fail if set -x is defined.

diff --git a/snf-image-helper/common.sh b/snf-image-helper/common.sh
index d666e4f..94b6bad 100644 (file)
--- a/snf-image-helper/common.sh
+++ b/snf-image-helper/common.sh
@@ -56,7 +56,8 @@ add_cleanup() {
 report_error() {
     if [ ${#ERRORS[*]} -eq 0 ]; then
         # No error message. Print stderr
-       local lines=$(tail --lines=${STDERR_LINE_SIZE} "$STDERR_FILE" | wc -l)
+        local lines
+        lines=$(tail --lines=${STDERR_LINE_SIZE} "$STDERR_FILE" | wc -l)
         echo -n "STDERR:${lines}:" > "$MONITOR"
         tail --lines=$lines  "$STDERR_FILE" > "$MONITOR"
     else
@@ -111,7 +112,8 @@ get_base_distro() {
 }
 
 get_distro() {
-    local root_dir=$1
+    local root_dir distro
+    root_dir=$1
 
     if [ -e "$root_dir/etc/debian_version" ]; then
         distro="debian"
@@ -143,7 +145,8 @@ get_distro() {
 
 
 get_partition_table() {
-    local dev="$1"
+    local dev output
+    dev="$1"
     # If the partition table is gpt then parted will raise an error if the
     # secondary gpt is not it the end of the disk, and a warning that has to
     # do with the "Last Usable LBA" entry in gpt.
@@ -155,10 +158,10 @@ get_partition_table() {
 }
 
 get_partition_table_type() {
-    local ptable="$1"
+    local ptable dev field
+    ptable="$1"
 
-    local dev="$(sed -n 2p <<< "$ptable")"
-    declare -a field
+    dev="$(sed -n 2p <<< "$ptable")"
     IFS=':' read -ra field <<< "$dev"
 
     echo "${field[5]}"
@@ -196,8 +199,9 @@ is_extended_partition() {
 }
 
 get_extended_partition() {
-    local ptable="$1"
-    local dev="$(echo "$ptable" | sed -n 2p | cut -d':' -f1)"
+    local ptable dev
+    ptable="$1"
+    dev="$(echo "$ptable" | sed -n 2p | cut -d':' -f1)"
 
     tail -n +3 <<< "$ptable" | while read line; do
         part_num=$(cut -d':' -f1 <<< "$line")
@@ -210,7 +214,8 @@ get_extended_partition() {
 }
 
 get_logical_partitions() {
-    local ptable="$1"
+    local ptable part_num
+    ptable="$1"
 
     tail -n +3 <<< "$ptable" | while read line; do
         part_num=$(cut -d':' -f1 <<< "$line")
@@ -223,8 +228,9 @@ get_logical_partitions() {
 }
 
 get_last_primary_partition() {
-    local ptable="$1"
-    local dev=$(echo "ptable" | sed -n 2p | cut -d':' -f1)
+    local ptable dev output
+    ptable="$1"
+    dev=$(echo "ptable" | sed -n 2p | cut -d':' -f1)
 
     for i in 4 3 2 1; do
         if output=$(grep "^$i:" <<< "$ptable"); then
@@ -236,7 +242,9 @@ get_last_primary_partition() {
 }
 
 get_partition_to_resize() {
-    local dev="$1"
+    local dev table table_type last_part last_part_num extended last_primary \
+        ext_num prim_num
+    dev="$1"
 
     table=$(get_partition_table "$dev")
 
@@ -269,7 +277,7 @@ create_partition() {
     local part="$2"
     local ptype="$3"
 
-    declare -a fields
+    local fields=()
     IFS=":;" read -ra fields <<< "$part"
     local id="${fields[0]}"
     local start="${fields[1]}"
@@ -286,20 +294,21 @@ create_partition() {
 }
 
 enlarge_partition() {
-    local device="$1"
-    local part="$2"
-    local ptype="$3"
-    local new_end="$4"
+    local device part ptype new_end fields new_part table logical id
+    device="$1"
+    part="$2"
+    ptype="$3"
+    new_end="$4"
 
     if [ -z "$new_end" ]; then
         new_end=$(cut -d: -f 3 <<< "$(get_last_free_sector "$device")")
     fi
 
-    declare -a fields
+    fields=()
     IFS=":;" read -ra fields <<< "$part"
     fields[2]="$new_end"
 
-    local new_part=""
+    new_part=""
     for ((i = 0; i < ${#fields[*]}; i = i + 1)); do
         new_part="$new_part":"${fields[$i]}"
     done
@@ -308,8 +317,8 @@ enlarge_partition() {
     # If this is an extended partition, removing it will also remove the
     # logical partitions it contains. We need to save them for later.
     if [ "$ptype" = "extended" ]; then
-        local table="$(get_partition_table "$device")"
-        local logical="$(get_logical_partitions "$table")"
+        table="$(get_partition_table "$device")"
+        logical="$(get_logical_partitions "$table")"
     fi
 
     id=${fields[0]}
@@ -325,15 +334,16 @@ enlarge_partition() {
 }
 
 get_last_free_sector() {
-    local dev="$1"
-    local unit="$2"
+    local dev unit last_line ptype
+    dev="$1"
+    unit="$2"
 
     if [ -n "$unit" ]; then
         unit="unit $unit"
     fi
 
-    local last_line="$("$PARTED" -s -m "$dev" "$unit" print free | tail -1)"
-    local ptype="$(cut -d: -f 5 <<< "$last_line")"
+    last_line="$($PARTED -s -m "$dev" "$unit" print free | tail -1)"
+    ptype="$(cut -d: -f 5 <<< "$last_line")"
 
     if [ "$ptype" = "free;" ]; then
         echo "$last_line"
@@ -341,7 +351,8 @@ get_last_free_sector() {
 }
 
 get_unattend() {
-    local target="$1"
+    local target exists
+    target="$1"
 
     # Workaround to search for $target/Unattend.xml in an case insensitive way.
     exists=$(find "$target"/ -maxdepth 1 -iname unattend.xml)
@@ -385,7 +396,7 @@ cleanup() {
 }
 
 task_cleanup() {
-    rc=$?
+    local rc=$?
 
     if [ $rc -eq 0 ]; then
        report_task_end
@@ -395,8 +406,9 @@ task_cleanup() {
 }
 
 check_if_excluded() {
-    local name="$(tr [a-z] [A-Z] <<< ${PROGNAME:2})"
-    local exclude="SNF_IMAGE_PROPERTY_EXCLUDE_TASK_${name}"
+    local name exclude
+    name="$(tr [a-z] [A-Z] <<< ${PROGNAME:2})"
+    exclude="SNF_IMAGE_PROPERTY_EXCLUDE_TASK_${name}"
     if [ -n "${!exclude}" ]; then
         warn "Task ${PROGNAME:2} was excluded and will not run."
         exit 0

diff --git a/snf-image-helper/tasks/50AssignHostname.in b/snf-image-helper/tasks/50AssignHostname.in
index d1bc9bf..944fe8f 100644 (file)
--- a/snf-image-helper/tasks/50AssignHostname.in
+++ b/snf-image-helper/tasks/50AssignHostname.in
@@ -34,15 +34,16 @@ report_task_start
 check_if_excluded
 
 windows_hostname() {
-    local target="$1"
-    local password="$2"
+    local target password unattend tmp_unattend namespace
+    target="$1"
+    password="$2"
 
-    local tmp_unattend=$(mktemp) || exit 1
+    tmp_unattend=$(mktemp)
     add_cleanup rm "$tmp_unattend"
 
     echo -n "Assigning new computer name..."
 
-    local namespace="urn:schemas-microsoft-com:unattend"
+    namespace="urn:schemas-microsoft-com:unattend"
 
     unattend=$(get_unattend "$target")
     if [ -z "$unattend" ]; then
@@ -56,10 +57,11 @@ windows_hostname() {
 }
 
 linux_hostname() {
-    local target="$1"
-    local hostname="$2"
+    local target hostname distro
+    target="$1"
+    hostname="$2"
 
-    local distro=$(get_base_distro "$target")
+    distro=$(get_base_distro "$target")
 
     case "$distro" in
         debian)

diff --git a/snf-image-helper/tasks/50ChangePassword.in b/snf-image-helper/tasks/50ChangePassword.in
index 54b3f76..375eeec 100644 (file)
--- a/snf-image-helper/tasks/50ChangePassword.in
+++ b/snf-image-helper/tasks/50ChangePassword.in
@@ -34,8 +34,9 @@ report_task_start
 check_if_excluded
 
 windows_password() {
-    local target="$1"
-    local password="$2"
+    local target password
+    target="$1"
+    password="$2"
 
     echo "@echo off" > "$target/Windows/SnfScripts/ChangeAdminPassword.cmd"
 
@@ -55,15 +56,16 @@ windows_password() {
 }
 
 linux_password() {
-    local target="$1"
-    local password="$2"
+    local target password hash users tmp_shadow
+    target="$1"
+    password="$2"
 
-    local hash=$("@scriptsdir@/snf-passtohash.py" "$password")
+    hash=$("@scriptsdir@/snf-passtohash.py" "$password")
     if [ ! -e "$target/etc/shadow" ]; then
        log_error "No /etc/shadow found!" 
     fi
     
-    declare -a users
+    users=()
     
     if [ -n "$SNF_IMAGE_PROPERTY_USERS" ]; then
         for usr in $SNF_IMAGE_PROPERTY_USERS; do
@@ -76,7 +78,7 @@ linux_password() {
     fi
 
     for i in $(seq 0 1 $((${#users[@]}-1))); do
-        local tmp_shadow="$(mktemp)"
+        tmp_shadow="$(mktemp)"
         add_cleanup rm "$tmp_shadow"
 
         echo -n "Setting ${users[$i]} password..."

diff --git a/snf-image-host/common.sh.in b/snf-image-host/common.sh.in
index 3510a3f..075dc94 100644 (file)
--- a/snf-image-host/common.sh.in
+++ b/snf-image-host/common.sh.in
@@ -51,10 +51,9 @@ report_error() {
 }
 
 report_info() {
+    local report
     echo "[INFO] $*" >&2
-
-    local report="$(./host-monitor.py info <<< "$*")"
-
+    report="$(./host-monitor.py info <<< "$*")"
     eval "echo $(printf "%q" "$report") >&${MONITOR_FD}"
 }
 
@@ -207,9 +206,11 @@ EOF
 }
 
 create_floppy() {
-    local img=$1
+    local img target
+
+    img=$1
 
-    local target=$(mktemp -d)
+    target=$(mktemp -d)
     add_cleanup rmdir "$target"
 
     dd bs=512 count=2880 if=/dev/zero of="$img"